Closed
Bug 1107963
Opened 10 years ago
Closed 10 years ago
crash in JS::PersistentRooted<JSObject*>::PersistentRooted<JSObject*>(JSContext*, JSObject*)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
firefox35 | --- | verified |
firefox36 | --- | unaffected |
firefox37 | --- | unaffected |
People
(Reporter: lizzard, Assigned: jonco)
Details
(Keywords: crash, topcrash)
Crash Data
Attachments
(1 file)
14.52 KB,
patch
|
terrence
:
feedback+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-c2974926-d138-4a4c-9ba7-091c62141201. ============================================================= This is a topcrasher for 34.0.5 and 34.0. It first appeared on 2014-04-18. crashing thread: 0 xul.dll JS::PersistentRooted<JSObject*>::PersistentRooted<JSObject*>(JSContext*, JSObject*) js/public/RootingAPI.h 1 xul.dll nsJSObjWrapper::nsJSObjWrapper(_NPP*) dom/plugins/base/nsJSNPRuntime.cpp 2 xul.dll nsJSObjWrapper::NP_Allocate(_NPP*, NPClass*) dom/plugins/base/nsJSNPRuntime.cpp 3 xul.dll mozilla::plugins::parent::_createobject(_NPP*, NPClass*) dom/plugins/base/nsNPAPIPlugin.cpp 4 xul.dll nsJSObjWrapper::GetNewOrUsed(_NPP*, JSContext*, JS::Handle<JSObject*>) dom/plugins/base/nsJSNPRuntime.cpp 5 xul.dll JSValToNPVariant(_NPP*, JSContext*, JS::Value, _NPVariant*) dom/plugins/base/nsJSNPRuntime.cpp 6 xul.dll nsJSObjWrapper::NP_GetProperty(NPObject*, void*, _NPVariant*) dom/plugins/base/nsJSNPRuntime.cpp 7 xul.dll mozilla::plugins::parent::_getproperty(_NPP*, NPObject*, void*, _NPVariant*) dom/plugins/base/nsNPAPIPlugin.cpp 8 xul.dll mozilla::plugins::PluginScriptableObjectParent::AnswerGetParentProperty(mozilla::plugins::PPluginIdentifierParent*, mozilla::plugins::Variant*, bool*) dom/plugins/ipc/PluginScriptableObjectParent.cpp 9 xul.dll mozilla::plugins::PPluginScriptableObjectParent::OnCallReceived(IPC::Message const&, IPC::Message*&) obj-firefox/ipc/ipdl/PPluginScriptableObjectParent.cpp 10 xul.dll mozilla::plugins::PPluginModuleParent::OnCallReceived(IPC::Message const&, IPC::Message*&) obj-firefox/ipc/ipdl/PPluginModuleParent.cpp 11 xul.dll mozilla::ipc::MessageChannel::DispatchInterruptMessage(IPC::Message const&, unsigned int) ipc/glue/MessageChannel.cpp 12 xul.dll mozilla::ipc::MessageChannel::InterruptCall(IPC::Message*, IPC::Message*) ipc/glue/MessageChannel.cpp 13 xul.dll mozilla::ipc::MessageChannel::Call(IPC::Message*, IPC::Message*) ipc/glue/MessageChannel.cpp 14 xul.dll mozilla::plugins::PPluginInstanceParent::CallNPP_Destroy(short*) obj-firefox/ipc/ipdl/PPluginInstanceParent.cpp 15 xul.dll mozilla::plugins::PluginInstanceParent::Destroy() dom/plugins/ipc/PluginInstanceParent.cpp
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to Liz Henry :lizzard from comment #0) It looks like we are passing null JSContext* when creating the PersistentRooted. I can't see exactly how this happens but I suspect it's because the plugin is being destroyed at the time. Note that use of PersistentRooted<JSObject*> in this code got removed in a change that landed in FF36 (changeset e5273d8dcb91 from bug 650161). I suppose oneoption is to backport the relevant parts of that change. I don't see any plugin topcrashers for 36 and 37.
Assignee | ||
Comment 2•10 years ago
|
||
Here's a backport to mozilla-beta of the relevant parts of that patch. Since this is not really my area, johns are you ok with this course of action?
Assignee: nobody → jcoppeard
Attachment #8534986 -
Flags: feedback?(john)
Comment 3•10 years ago
|
||
Comment on attachment 8534986 [details] [diff] [review] bug1107963-plugins-crash bsmedberg should probably make the call on the riskiness of this, but I don't think this is an issue as far as the plugin bits go. The JS-GC bits are voodoo to me, so I can't comment there.
Attachment #8534986 -
Flags: feedback?(john) → feedback?(benjamin)
Comment 4•10 years ago
|
||
Comment on attachment 8534986 [details] [diff] [review] bug1107963-plugins-crash Plugin changes are inherently risky. If this is a straight backport of something that's already landed, that reduces the risk. I'm going to bounce this back to somebody who hopefully understands it.
Attachment #8534986 -
Flags: feedback?(benjamin) → feedback?(terrence)
Comment 5•10 years ago
|
||
Comment on attachment 8534986 [details] [diff] [review] bug1107963-plugins-crash Review of attachment 8534986 [details] [diff] [review]: ----------------------------------------------------------------- The new code is well tested at this point and it doesn't look like any of the surrounding code has changed -- let's uplift this.
Attachment #8534986 -
Flags: feedback?(terrence) → feedback+
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8534986 [details] [diff] [review] bug1107963-plugins-crash Approval Request Comment [Feature/regressing bug #]: Bug 993413 [User impact if declined]: Occasional crashes [Describe test coverage new/current, TBPL]: This is an uplift of a change that landed on central over a month ago (in changeset e5273d8dcb91) [Risks and why]: Low [String/UUID change made/needed]: None
Attachment #8534986 -
Flags: approval-mozilla-beta?
Updated•10 years ago
|
Attachment #8534986 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 7•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/b7290de61970
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox35:
--- → fixed
status-firefox36:
--- → unaffected
status-firefox37:
--- → unaffected
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 8•9 years ago
|
||
Socorro [1] shows zero crashes in Firefox 35 Beta 6 & 8. [1] - https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=JS%3A%3APersistentRooted%3CJSObject%2A%3E%3A%3APersistentRooted%3CJSObject%2A%3E%28JSContext%2A%2C+JSObject%2A%29
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•