Assertion failure: cache->GetWrapperMaybeDead() == obj, at src/obj-firefox/dist/include/mozilla/dom/BindingUtils.h:1389
Categories
(Core :: DOM: CSS Object Model, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr60 | --- | unaffected |
| firefox-esr68 | --- | wontfix |
| firefox68 | --- | wontfix |
| firefox69 | --- | wontfix |
| firefox70 | --- | fixed |
People
(Reporter: tsmith, Assigned: bzbarsky)
References
(Blocks 2 open bugs)
Details
(Keywords: assertion, regression, testcase)
Attachments
(2 files)
Not sure what the correct component is so let's start with GC.
STR:
- unpack test.zip
- using a fuzzing build, a clean profile and the included prefs.js launch the browser
- open launcher.html
- wait 30 - 45 seconds
Marking as s-s to be safe.
I can consistently reproduce the issue with a fuzzing debug build.
Assertion failure: cache->GetWrapperMaybeDead() == obj, at /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/BindingUtils.h:1389
0|0|libxul.so|void mozilla::dom::ClearWrapper<mozilla::dom::CSSFontFeatureValuesRule>(mozilla::dom::CSSFontFeatureValuesRule*, nsWrapperCache*, JSObject*)|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.h:d6eed298a41b1536a7f8624a5a4728847ac0b77a|1389|0x16
0|1|libxul.so|mozilla::dom::CSS2Properties_Binding::DOMProxyHandler::finalize(JSFreeOp*, JSObject*) const|s3:gecko-generated-sources:323373662f269fc3be6ef40cb980a8dc933b37a761a91ee9b85133fd8d4c40fb8382a8a2ccd7a3c3a682b232eae1dcd81c33058d80181545eb61f2e6fe39b989/dom/bindings/CSS2PropertiesBinding.cpp:|55210|0xc
0|2|libxul.so|proxy_Finalize|hg:hg.mozilla.org/mozilla-central:js/src/proxy/Proxy.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|724|0x23
0|3|libxul.so|FinalizeTypedArenas<JSObject>|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|653|0x4aa
0|4|libxul.so|FinalizeArenas|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|681|0x9f0
0|5|libxul.so|js::gc::ArenaLists::foregroundFinalize(js::FreeOp*, js::gc::AllocKind, js::SliceBudget&, js::gc::SortedArenaList&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6070|0x1f
0|6|libxul.so|js::gc::GCRuntime::finalizeAllocKind(js::FreeOp*, js::SliceBudget&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6362|0x34
0|7|libxul.so|sweepaction::SweepActionForEach<ContainerIter<mozilla::EnumSet<js::gc::AllocKind, unsigned int> >, mozilla::EnumSet<js::gc::AllocKind, unsigned int> >::run(js::gc::SweepAction::Args&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6588|0x16
0|8|libxul.so|sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6553|0x11
0|9|libxul.so|sweepaction::SweepActionForEach<js::gc::SweepGroupZonesIter, JSRuntime*>::run(js::gc::SweepAction::Args&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6588|0x12
0|10|libxul.so|sweepaction::SweepActionSequence::run(js::gc::SweepAction::Args&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6553|0x11
0|11|libxul.so|sweepaction::SweepActionForEach<js::gc::SweepGroupsIter, JSRuntime*>::run(js::gc::SweepAction::Args&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6588|0x16
0|12|libxul.so|js::gc::GCRuntime::performSweepActions(js::SliceBudget&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|6722|0x28
0|13|libxul.so|js::gc::GCRuntime::incrementalSlice(js::SliceBudget&, JS::GCReason, js::gc::AutoGCSession&)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|7251|0xb
0|14|libxul.so|js::gc::GCRuntime::gcCycle(bool, js::SliceBudget, JS::GCReason)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|7631|0x16
0|15|libxul.so|js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::GCReason)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|7811|0x1c
0|16|libxul.so|js::gc::GCRuntime::gc(JSGCInvocationKind, JS::GCReason)|hg:hg.mozilla.org/mozilla-central:js/src/gc/GC.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|7899|0x21
0|17|libxul.so|nsJSContext::GarbageCollectNow(JS::GCReason, nsJSContext::IsIncremental, nsJSContext::IsShrinking, long)|hg:hg.mozilla.org/mozilla-central:dom/base/nsJSEnvironment.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|1137|0xe
0|18|libxul.so|mozilla::dom::FuzzingFunctions_Binding::garbageCollect|s3:gecko-generated-sources:299a263fb9d986b6d25517d97e304a8454f1163f23c5536904ab3c7dee1bf1c02917adc3d237e26ca0f7f200d3429207e84b4cc4132d6c8360b36bfbb2e64081/dom/bindings/FuzzingFunctionsBinding.cpp:|42|0x8
0|19|libxul.so|CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|448|0x16
0|20|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|540|0x12
0|21|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|595|0xd
0|22|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|599|0x13
0|23|libxul.so|js::RunScript(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|425|0xb
0|24|libxul.so|js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|787|0x5
0|25|libxul.so|js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|821|0x5
0|26|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/vm/CompilationAndEvaluation.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|448|0x12
0|27|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/vm/CompilationAndEvaluation.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|468|0x5
0|28|libxul.so|nsJSUtils::ExecutionContext::ExecScript()|hg:hg.mozilla.org/mozilla-central:dom/base/nsJSUtils.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|416|0x14
0|29|libxul.so|WindowScriptTimeoutHandler::Call(char const*)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowInner.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|5719|0x8
0|30|libxul.so|nsGlobalWindowInner::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*)|hg:hg.mozilla.org/mozilla-central:dom/base/nsGlobalWindowInner.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|5927|0x14
0|31|libxul.so|mozilla::dom::TimeoutManager::RunTimeout(mozilla::TimeStamp const&, mozilla::TimeStamp const&, bool)|hg:hg.mozilla.org/mozilla-central:dom/base/TimeoutManager.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|971|0x13
0|32|libxul.so|mozilla::dom::TimeoutExecutor::MaybeExecute()|hg:hg.mozilla.org/mozilla-central:dom/base/TimeoutExecutor.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|179|0x13
0|33|libxul.so|mozilla::dom::TimeoutExecutor::Notify(nsITimer*)|hg:hg.mozilla.org/mozilla-central:dom/base/TimeoutExecutor.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|246|0x5
0|34|libxul.so|nsTimerImpl::Fire(int)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsTimerImpl.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|564|0xe
0|35|libxul.so|nsTimerEvent::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/TimerThread.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|260|0x18
0|36|libxul.so|mozilla::ThrottledEventQueue::Inner::ExecuteRunnable()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/ThrottledEventQueue.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|252|0x11
0|37|libxul.so|mozilla::ThrottledEventQueue::Inner::Executor::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/ThrottledEventQueue.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|80|0xd
0|38|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|295|0x15
0|39|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|1225|0x15
0|40|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|486|0x11
0|41|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|88|0xa
0|42|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d6eed298a41b1536a7f8624a5a4728847ac0b77a|315|0x17
0|43|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d6eed298a41b1536a7f8624a5a4728847ac0b77a|290|0x8
0|44|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|137|0xd
0|45|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|919|0x11
0|46|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|238|0x5
0|47|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d6eed298a41b1536a7f8624a5a4728847ac0b77a|315|0x17
0|48|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d6eed298a41b1536a7f8624a5a4728847ac0b77a|290|0x8
0|49|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|754|0xc
0|50|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|56|0x14
0|51|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:d6eed298a41b1536a7f8624a5a4728847ac0b77a|267|0x12
0|52|libc-2.27.so||||0x21b97
0|53|firefox-bin|MOZ_ReportCrash|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:d6eed298a41b1536a7f8624a5a4728847ac0b77a|184|0x5
|
||
Comment 1•5 years ago
|
||
Probably a wrapper caching issue for CSSFontFeatureValuesRule.
|
Assignee | |
Comment 2•5 years ago
|
||
Lies and the lying debuggers that tell them about optimized builds... ;)
ClearWrapper<mozilla::dom::CSSFontFeatureValuesRule> is not going to get called by CSS2Properties_Binding::DOMProxyHandler::finalize, but likely COMDAT folding or equivalent is happening.
We're actually in ClearWrapper<nsDOMCSSDeclaration>, which makes a heck of a lot more sense given the caller. The actual object we have is a CSSStyleRuleDeclaration.
cache->GetWrapperMaybeDead() is nullptr. It got nulled because a memory-pressure observer triggered CC, which landed in CSSStyleRule::cycleCollection::Unlink via nsCycleCollector::CollectWhite. That did nsWrapperCache::ReleaseWrapper on the rule, which called into CycleCollectedJSRuntime::RemoveJSHolder (via DropJSObjectsImpl) which ended up calling into CSSStyleRule::cycleCollection::Trace which then did:
tmp->mDecls.TraceWrapper(aCallbacks, aClosure);
and tmp.mDecls is the CSSStyleRuleDeclaration. That clears out its mWrapper, because that's what ClearJSHolder::Trace (the thing that RemoveJSHolder ends up tracing with) does: clears out things.
Now why doesn't this blow up in our faces all the time? Well, normally say you have a wrapper-cached object X. When X is being unlinked, X::ReleaseWrapper happens, we stop preserving the wrapper, then do the DropJSObjectImpl bit. We land in the trace method for X, which calls nsWrapperCache::TraceWrapper, which sees that we are not preserving the wrapper and does not actually trace it. Which means it does NOT get cleared out by this ClearJSHolder thing normally.
The CSSStyleRule situation is a bit weird because there are two wrapper caches involved: one for the rule, and one for the declaration. But there is only one object, really: the declaration is a by-value member of the rule. There's only one node in the CC graph involved. So we land in CSSStyleRule's unlink, call the superclass unlink, that does NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER, etc, but the CSSStyleRuleDeclaration is still preserving its wrapper (hasn't been unlinked yet), so when we do the tracing thing we end up in this situation.
I think the right solution is for CSSStyleRule unlink to unlink mDecls before unlinking the superclass, but I'm open to other bright ideas.
|
|
Assignee | |
Comment 3•5 years ago
|
||
Testcase that reproduces the assertion in a non-fuzzing build without any special prefs if you load it and then trigger memory minimization in about:memory:
<style>
x { }
</style>
<script>
var sheet = document.styleSheets[0];
var rule = sheet.cssRules[0];
var decl = rule.style;
rule.expando = 5;
decl.expando = 6;
sheet.deleteRule(0);
rule = decl = null;
</script>
Note that we need the rule to be preserving its wrapper, so that its ReleaseWrapper will do the DropJSObjectsImpl thing, and we want the decl to be preserving its wrapper, so when we trace it with ClearJSHolder that's not a no-op.
By the way, I think this is not a security problem. The assert is failing, but we don't have a "wrong" wrapper in the sense of having one that is non-null but different from the thing being finalized. We are in a state where the C++ object thinks it has no wrapper, but it's also been unlinked, so it seems pretty unlikely that it will create a new wrapper, right?
It'd still be good to fix this, though.
|
||
Comment 4•5 years ago
|
||
I can take a poke at fixing it, thanks Boris for diagnosing :)
|
|
||
Updated•5 years ago
|
|
|
||
Comment 5•5 years ago
|
||
|
|
Assignee | |
Comment 6•5 years ago
|
||
In general, though, I don't understand this "trace with ClearJSHolder, but only if we used to be preserving our wrapper" thing. As described above, the ClearJSHolder doesn't affect the wrapper itself no matter what. So presumably this is being done to clear other JS refs that the object might be holding? But then why are we only doing that if it's preserving its wrapper? Seems like in general those refs would be independent of the wrapper preservation state.
It looks like CycleCollectedJSRuntime::RemoveJSHolder is sort of doing double duty. It's being used both to remove things from the holder hash and to clear out any refs they might have. The latter makes sense for various manual mozilla::DropJSObjects callers, probably. But not for the specific wrappercache case, really; there we could just remove from the hashtable without doing the trace, afaict. Worth doing?
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
||
Comment 7•5 years ago
|
||
Per comment #3:
By the way, I think this is not a security problem. The assert is failing, but we don't have a "wrong" wrapper in the sense of having one that is non-null but different from the thing being finalized. We are in a state where the C++ object thinks it has no wrapper, but it's also been unlinked, so it seems pretty unlikely that it will create a new wrapper, right?
I think this is not a security-sensitive bug, should I just land the patch?
|
|
||
Comment 8•5 years ago
|
||
|
|
||
Updated•5 years ago
|
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/54e5c7e82504 Make sure to unlink rule subclass members before unlinking css::Rule. r=bzbarsky
|
||
Comment 10•5 years ago
|
||
Backed out changeset 54e5c7e82504 (bug 1566684) for crashtest failures at dist/include/mozilla/dom/BindingUtils.h
Backout: https://hg.mozilla.org/integration/autoland/rev/eccbbb296d444552cf497e4df57e43bd87aa695b
Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=54e5c7e82504eef2244f7d34432cf662b820eb23
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256967000&repo=autoland&lineNumber=116085
|
||
Comment 11•5 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #6)
But not for the specific wrappercache case, really; there we could just remove from the hashtable without doing the trace, afaict. Worth doing?
But the object could keep other js things alive, and we want to clear those references if the object stops being a jsholder.
|
|
Assignee | |
Comment 12•5 years ago
|
||
But the object could keep other js things alive, and we want to clear those references if the object stops being a jsholder.
Hmm. I guess the point is that it could be a holder both because it's got a preserved wrapper and because it also called HoldJSObjects elsewhere for other reference it holds? And then unpreserving the wrapper will also implicitly make it stop being a holder for those other objects (because it will remove it from the holder hashtable), and then we need to clear those references, because otherwise they could go stale?
|
|
||
Comment 13•5 years ago
|
||
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #12)
And then unpreserving the wrapper will also implicitly make it stop being a holder for those other objects (because it will remove it from the holder hashtable), and then we need to clear those references, because otherwise they could go stale?
Yes, that sounds right to me.
|
|
||
Comment 14•5 years ago
|
||
So with this change, the failure is slighty different, in the sense that now instead of in the declarations we assert on the rule. The wrapper we assert on is cleared from:
#0 0x00007f1f531c4f98 in ClearJSHolder::Trace(JSObject**, char const*, void*) const (this=0x7fffc7ea8a30, aPtr=0x7f1f3af91610, aName=0x7f1f4f2fbc97 "Preserved wrapper", aClosure=0x0)
at /home/emilio/src/moz/gecko-3/xpcom/base/CycleCollectedJSRuntime.cpp:1035
#1 0x00007f1f534dc1a6 in nsWrapperCache::TraceWrapper(TraceCallbacks const&, void*) (this=0x7f1f3af91608, aCallbacks=..., aClosure=0x0) at /home/emilio/src/moz/gecko-3/obj-debug-noopt/dist/include/nsWrapperCache.h:255
#2 0x00007f1f58be83ed in mozilla::css::Rule::cycleCollection::Trace(void*, TraceCallbacks const&, void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600, aCallbacks=..., aClosure=0x0)
at /home/emilio/src/moz/gecko-3/layout/style/Rule.cpp:31
#3 0x00007f1f58b782e4 in mozilla::dom::CSSStyleRule::cycleCollection::Trace(void*, TraceCallbacks const&, void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600, aCallbacks=..., aClosure=0x0)
at /home/emilio/src/moz/gecko-3/layout/style/CSSStyleRule.cpp:106
#4 0x00007f1f53192104 in mozilla::CycleCollectedJSRuntime::RemoveJSHolder(void*) (this=0x7f1f3de2c000, aHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/xpcom/base/CycleCollectedJSRuntime.cpp:1064
#5 0x00007f1f53199d59 in mozilla::cyclecollector::DropJSObjectsImpl(void*) (aHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/xpcom/base/HoldDropJSObjects.cpp:35
#6 0x00007f1f5583cb41 in nsWrapperCache::ReleaseWrapper(void*) (this=0x7f1f3af91660, aScriptObjectHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/dom/base/nsWrapperCache.cpp:50
#7 0x00007f1f58b7832d in mozilla::dom::CSSStyleRule::cycleCollection::Unlink(void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600)
at /home/emilio/src/moz/gecko-3/layout/style/CSSStyleRule.cpp:116
#8 0x00007f1f531e8d0a in nsCycleCollector::CollectWhite() (this=0x7f1f3eed45e0) at /home/emilio/src/moz/gecko-3/xpcom/base/nsCycleCollector.cpp:3090
#9 0x00007f1f531ea15f in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) (this=0x7f1f3eed45e0, aCCType=ManualCC, aBudget=..., aManualListener=0x0, aPreferShorterSlices=false)
at /home/emilio/src/moz/gecko-3/xpcom/base/nsCycleCollector.cpp:3436
#10 0x00007f1f531ec894 in nsCycleCollector_collect(nsICycleCollectorListener*) (aManualListener=0x0) at /home/emilio/src/moz/gecko-3/xpcom/base/nsCycleCollector.cpp:3949
#11 0x00007f1f55aabb02 in nsJSContext::CycleCollectNow(nsICycleCollectorListener*) (aListener=0x0) at /home/emilio/src/moz/gecko-3/dom/base/nsJSEnvironment.cpp:1423
#12 0x00007f1f54822bc9 in nsXPCComponents_Utils::ForceCC(nsICycleCollectorListener*) (this=0x7f1f3be6aaf0, listener=0x0) at /home/emilio/src/moz/gecko-3/js/xpconnect/src/XPCComponents.cpp:1624
#0 0x00007f1f531c4f98 in ClearJSHolder::Trace(JSObject**, char const*, void*) const (this=0x7fffc7ea8a30, aPtr=0x7f1f3af91610, aName=0x7f1f4f2fbc97 "Preserved wrapper", aClosure=0x0)
at /home/emilio/src/moz/gecko-3/xpcom/base/CycleCollectedJSRuntime.cpp:1035
#1 0x00007f1f534dc1a6 in nsWrapperCache::TraceWrapper(TraceCallbacks const&, void*) (this=0x7f1f3af91608, aCallbacks=..., aClosure=0x0) at /home/emilio/src/moz/gecko-3/obj-debug-noopt/dist/include/nsWrapperCache.h:255
#2 0x00007f1f58be83ed in mozilla::css::Rule::cycleCollection::Trace(void*, TraceCallbacks const&, void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600, aCallbacks=..., aClosure=0x0)
at /home/emilio/src/moz/gecko-3/layout/style/Rule.cpp:31
#3 0x00007f1f58b782e4 in mozilla::dom::CSSStyleRule::cycleCollection::Trace(void*, TraceCallbacks const&, void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600, aCallbacks=..., aClosure=0x0)
at /home/emilio/src/moz/gecko-3/layout/style/CSSStyleRule.cpp:106
#4 0x00007f1f53192104 in mozilla::CycleCollectedJSRuntime::RemoveJSHolder(void*) (this=0x7f1f3de2c000, aHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/xpcom/base/CycleCollectedJSRuntime.cpp:1064
#5 0x00007f1f53199d59 in mozilla::cyclecollector::DropJSObjectsImpl(void*) (aHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/xpcom/base/HoldDropJSObjects.cpp:35
#6 0x00007f1f5583cb41 in nsWrapperCache::ReleaseWrapper(void*) (this=0x7f1f3af91660, aScriptObjectHolder=0x7f1f3af91600) at /home/emilio/src/moz/gecko-3/dom/base/nsWrapperCache.cpp:50
#7 0x00007f1f58b7832d in mozilla::dom::CSSStyleRule::cycleCollection::Unlink(void*) (this=0x7f1f5fb1e118 <mozilla::dom::CSSStyleRule::_cycleCollectorGlobal>, p=0x7f1f3af91600)
at /home/emilio/src/moz/gecko-3/layout/style/CSSStyleRule.cpp:116
#8 0x00007f1f531e8d0a in nsCycleCollector::CollectWhite() (this=0x7f1f3eed45e0) at /home/emilio/src/moz/gecko-3/xpcom/base/nsCycleCollector.cpp:3090
#9 0x00007f1f531ea15f in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) (this=0x7f1f3eed45e0, aCCType=ManualCC, aBudget=..., aManualListener=0x0, aPreferShorterSlices=false)
at /home/emilio/src/moz/gecko-3/xpcom/base/nsCycleCollector.cpp:3436
So at this point I'm not sure on the best path forward... It seems like we end up tracing the rule object from unlink, which does also clear the wrapper, and then the gc gets confused... Switching the tracing order also unsurprisingly doesn't help. Boris, any ideas other than not making the assertion fire with a null wrapper?
|
|
Assignee | |
Comment 15•5 years ago
|
||
So basically in the new setup we unlink the wrappercache on decls first, which does a trace and clears the rule's wrapper? I I should have realized that.
I started trying to write down how I think we should fix this, then realized I was just translating code I had in my head into English and it would be faster to write the code. Stealing.
|
|
||
Updated•5 years ago
|
|
||
Comment 16•5 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00624a27a279 Make sure to unlink rule declarations before unlinking css::Rule. r=smaug,emilio
|
|
||
Comment 17•5 years ago
|
||
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c1695ee714a8 followup to address review comments. r=smaug, DONTBUILD
|
||
Comment 18•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/00624a27a279
https://hg.mozilla.org/mozilla-central/rev/c1695ee714a8
|
||
Comment 19•5 years ago
|
||
Is there a user impact here which justifies backport consideration or can this fix ride the trains?
|
|
Assignee | |
Comment 20•5 years ago
|
||
I think this can ride the trains; I can't offhand think of a problem this causes in non-debug builds.
|
|
||
Updated•5 years ago
|
|
||
Comment 21•5 years ago
|
||
Bugbug thinks this bug is a regression, but please revert this change in case of error.
|
|
Assignee | |
Comment 22•5 years ago
|
||
I'd love to know why bugbug thinks that. As far as I can tell, it's not, really, unless you count the point when the assertion got added...
|
||
Comment 23•5 years ago
|
||
bugbug is based on machine learning, so there is a margin of error. I'm guessing the decision in this case was based on the status flags (when an earlier version is set as unaffected and a newer version is set as affected, it's usually a strong indication of a regression).
Description
•