Bug 186223

Summary: LayoutTests/fast/css/parsing-css-matches-7.html always abandons its Document (disabling JIT fixes it)
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, ews-watchlist, fpizlo, ggaren, keith_miller, mark.lam, msaboff, saam, simon.fraser, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Simon Fraser (smfr)
Reported 2018-06-01 21:29:38 PDT
After running LayoutTests/fast/css/parsing-css-matches-7.html the Document never goes away.
Attachments
Patch (13.75 KB, patch)
2018-06-02 16:39 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2018-06-01 22:36:17 PDT
Options::useConcurrentGC() = false does NOT fix the bug. Options::useJIT() = false DOES fix the bug.
Simon Fraser (smfr)
Comment 2 2018-06-01 22:37:27 PDT
Oddly almost any change simplify the test makes the bug go away.
Simon Fraser (smfr)
Comment 3 2018-06-01 22:38:55 PDT
GC heap dump doesn't show anything interesting, other than the fact that the JSHTMLDocument is kept alive by being visited from the JSGlobalObject (and thus the HTMLDocument lives).
Radar WebKit Bug Importer
Comment 4 2018-06-01 23:12:46 PDT
<rdar://problem/40744972>
Simon Fraser (smfr)
Comment 5 2018-06-02 10:03:10 PDT
Disabling the DFGJIT also fixes this. With the DFGJIT disabled, the document is released via: * frame #0: 0x000000010d3b49e6 WebCore`WebCore::Document::~Document(this=0x000000012a200fb8) at Document.cpp:585 frame #1: 0x000000010d757d95 WebCore`WebCore::HTMLDocument::~HTMLDocument(this=0x000000012a200fb8) at HTMLDocument.cpp:95 frame #2: 0x000000010d757db5 WebCore`WebCore::HTMLDocument::~HTMLDocument(this=0x000000012a200fb8) at HTMLDocument.cpp:95 frame #3: 0x000000010d757e59 WebCore`WebCore::HTMLDocument::~HTMLDocument(this=0x000000012a200fb8) at HTMLDocument.cpp:95 frame #4: 0x000000010d3b8300 WebCore`WebCore::Document::decrementReferencingNodeCount(this=0x000000012a200fb8) at Document.h:361 frame #5: 0x000000010d4e7e60 WebCore`WebCore::Node::~Node(this=0x0000000130d03db0) at Node.cpp:314 frame #6: 0x000000010d36c087 WebCore`WebCore::ContainerNode::~ContainerNode(this=0x0000000130d03db0) at ContainerNode.cpp:270 frame #7: 0x000000010d46001c WebCore`WebCore::Element::~Element(this=0x0000000130d03db0) at Element.cpp:199 frame #8: 0x000000010d579222 WebCore`WebCore::StyledElement::~StyledElement(this=0x0000000130d03db0) at StyledElement.cpp:66 frame #9: 0x000000010b8db1d5 WebCore`WebCore::HTMLElement::~HTMLElement(this=0x0000000130d03db0) at HTMLElement.h:38 frame #10: 0x000000010d8783c5 WebCore`WebCore::HTMLSpanElement::~HTMLSpanElement(this=0x0000000130d03db0) at HTMLSpanElement.h:32 frame #11: 0x000000010d8710c5 WebCore`WebCore::HTMLSpanElement::~HTMLSpanElement(this=0x0000000130d03db0) at HTMLSpanElement.h:32 frame #12: 0x000000010d8710e9 WebCore`WebCore::HTMLSpanElement::~HTMLSpanElement(this=0x0000000130d03db0) at HTMLSpanElement.h:32 frame #13: 0x000000010d4e85bb WebCore`WebCore::Node::removedLastRef(this=0x0000000130d03db0) at Node.cpp:2557 frame #14: 0x000000010d4e852c WebCore`WebCore::Node::deref(this=0x0000000130d03db0) at Node.cpp:365 frame #15: 0x000000010d4eaf55 WebCore`WebCore::Node::derefEventTarget(this=0x0000000130d03db0) at Node.cpp:817 frame #16: 0x000000010b8fe236 WebCore`WebCore::EventTarget::deref(this=0x0000000130d03db0) at EventTarget.h:64 frame #17: 0x000000010b8fe20f WebCore`WTF::Ref<WebCore::EventTarget, WTF::DumbPtrTraits<WebCore::EventTarget> >::~Ref(this=0x000000012f4147b8) at Ref.h:61 frame #18: 0x000000010b8ebdf5 WebCore`WTF::Ref<WebCore::EventTarget, WTF::DumbPtrTraits<WebCore::EventTarget> >::~Ref(this=0x000000012f4147b8) at Ref.h:55 frame #19: 0x000000010bd7a619 WebCore`WebCore::JSDOMWrapper<WebCore::EventTarget>::~JSDOMWrapper(this=0x000000012f4147a0) at JSDOMWrapper.h:72 frame #20: 0x000000010bd7a5f5 WebCore`WebCore::JSEventTarget::~JSEventTarget(this=0x000000012f4147a0) at JSEventTarget.h:31 frame #21: 0x000000010bd70f85 WebCore`WebCore::JSEventTarget::~JSEventTarget(this=0x000000012f4147a0) at JSEventTarget.h:31 frame #22: 0x000000010bd6d86d WebCore`WebCore::JSEventTarget::destroy(cell=0x000000012f4147a0) at JSEventTarget.cpp:227 frame #23: 0x0000000101edceaa JavaScriptCore`JSC::JSDestructibleObjectDestroyFunc::operator(this=0x00007ffeefbfe0e0, (null)=0x0000000126f00000, cell=0x000000012f4147a0)(JSC::VM&, JSC::JSCell*) const at JSDestructibleObjectHeapCellType.cpp:37 frame #24: 0x0000000101f119f5 JavaScriptCore`void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::JSDestructibleObjectDestroyFunc>(this=0x00007ffeefbfdfb0, cell=0x000000012f4147a0)::'lambda'(void*)::operator()(void*) const at MarkedBlockInlines.h:255 frame #25: 0x0000000101f0c566 JavaScriptCore`void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::JSDestructibleObjectDestroyFunc>(this=0x0000000130ace550, freeList=0x0000000000000000, emptyMode=IsEmpty, sweepMode=SweepOnly, destructionMode=BlockHasDestructors, scribbleMode=Scribble, newlyAllocatedMode=DoesNotHaveNewlyAllocated, marksMode=MarksStale, destroyFunc=0x00007ffeefbfe0e0) at MarkedBlockInlines.h:289 frame #26: 0x0000000101edce40 JavaScriptCore`void JSC::MarkedBlock::Handle::finishSweepKnowingHeapCellType<JSC::JSDestructibleObjectDestroyFunc>(this=0x0000000130ace550, freeList=0x0000000000000000, destroyFunc=0x00007ffeefbfe0e0) at MarkedBlockInlines.h:434 frame #27: 0x0000000101edcd08 JavaScriptCore`JSC::JSDestructibleObjectHeapCellType::finishSweep(this=0x00000001226fa100, handle=0x0000000130ace550, freeList=0x0000000000000000) at JSDestructibleObjectHeapCellType.cpp:52 frame #28: 0x0000000101a907c6 JavaScriptCore`JSC::Subspace::finishSweep(this=0x00000001270f7e10, block=0x0000000130ace550, freeList=0x0000000000000000) at Subspace.cpp:65 frame #29: 0x0000000101a75387 JavaScriptCore`JSC::MarkedBlock::Handle::sweep(this=0x0000000130ace550, freeList=0x0000000000000000) at MarkedBlock.cpp:432 frame #30: 0x00000001019ff194 JavaScriptCore`JSC::BlockDirectory::sweep(this=0x00007ffeefbfe2d0, index=10)::$_9::operator()(unsigned long) const at BlockDirectory.cpp:297 frame #31: 0x00000001019fbd0c JavaScriptCore`void WTF::FastBitVectorImpl<WTF::FastBitVectorWordOwner>::forEachSetBit<JSC::BlockDirectory::sweep(this=0x0000000122650ce8, func=0x00007ffeefbfe2d0)::$_9>(JSC::BlockDirectory::sweep()::$_9 const&) const at FastBitVector.h:347 frame #32: 0x00000001019fbc89 JavaScriptCore`JSC::BlockDirectory::sweep(this=0x0000000122650c60) at BlockDirectory.cpp:294 frame #33: 0x0000000101a83299 JavaScriptCore`JSC::MarkedSpace::sweep(this=0x00007ffeefbfe350, directory=0x0000000122650c60)::$_9::operator()(JSC::BlockDirectory&) const at MarkedSpace.cpp:236 frame #34: 0x0000000101a7795f JavaScriptCore`void JSC::MarkedSpace::forEachDirectory<JSC::MarkedSpace::sweep()::$_9>(this=0x0000000126f00138, functor=0x00007ffeefbfe350)::$_9 const&) at MarkedSpace.h:236 frame #35: 0x0000000101a77915 JavaScriptCore`JSC::MarkedSpace::sweep(this=0x0000000126f00138) at MarkedSpace.cpp:234 frame #36: 0x0000000101a1209a JavaScriptCore`JSC::Heap::sweepSynchronously(this=0x0000000126f00040) at Heap.cpp:1019 frame #37: 0x0000000101a125b1 JavaScriptCore`JSC::Heap::collectNow(this=0x0000000126f00040, synchronousness=Sync, request=GCRequest @ 0x00007ffeefbfe450) at Heap.cpp:1060 frame #38: 0x000000010ceba470 WebCore`WebCore::GCController::garbageCollectNow(this=0x00000001101c7900) at GCController.cpp:98 frame #39: 0x00000001198b7d8d WebKitLegacy`::+[WebCoreStatistics garbageCollectJavaScriptObjects](self=WebCoreStatistics, _cmd="garbageCollectJavaScriptObjects") at WebCoreStatistics.mm:115 frame #40: 0x000000010002259c DumpRenderTree`runTest(inputLine="/Volumes/Data/Development/apple/webkit/OpenSource/LayoutTests/fast/css/parsing-css-matches-8.html") at DumpRenderTree.mm:2100 frame #41: 0x000000010001f483 DumpRenderTree`dumpRenderTree(argc=3, argv=0x00007ffeefbff5e8) at DumpRenderTree.mm:1277 frame #42: 0x0000000100022e72 DumpRenderTree`DumpRenderTreeMain(argc=3, argv=0x00007ffeefbff5e8) at DumpRenderTree.mm:1398 frame #43: 0x00000001000a7472 DumpRenderTree`main(argc=3, argv=0x00007ffeefbff5e8) at DumpRenderTreeMain.mm:34 frame #44: 0x00007fff5b8d9015 libdyld.dylib`start + 1 frame #45: 0x00007fff5b8d9015 libdyld.dylib`start + 1
Simon Fraser (smfr)
Comment 6 2018-06-02 12:33:43 PDT
When the DFGJIT is enabled, the JSDOMWindow is treated as a conservative root via gatherScratchBufferRoots().
Simon Fraser (smfr)
Comment 7 2018-06-02 12:50:27 PDT
The relevant scratch buffer is a size 24 buffer allocated via: (lldb) bt * thread #61, name = 'WTF::AutomaticThread', stop reason = breakpoint 10.1 * frame #0: 0x000000011629d6c6 JavaScriptCore`JSC::VM::scratchBufferForSize(this=0x0000000129000a00, size=24) at VM.cpp:1220 frame #1: 0x00000001156d7cf0 JavaScriptCore`JSC::DFG::JITCompiler::makeCatchOSREntryBuffer(this=0x00007000034b62a0) at DFGJITCompiler.cpp:696 frame #2: 0x00000001156d84d5 JavaScriptCore`JSC::DFG::JITCompiler::compileFunction(this=0x00007000034b62a0) at DFGJITCompiler.cpp:427 frame #3: 0x000000011580559d JavaScriptCore`JSC::DFG::Plan::compileInThreadImpl(this=0x0000000128f73bc0) at DFGPlan.cpp:386 frame #4: 0x0000000115802bf2 JavaScriptCore`JSC::DFG::Plan::compileInThread(this=0x0000000128f73bc0, threadData=0x0000000124d00560) at DFGPlan.cpp:187 frame #5: 0x0000000115a6d25c JavaScriptCore`JSC::DFG::Worklist::ThreadBody::work(this=0x0000000124d00580) at DFGWorklist.cpp:111 frame #6: 0x0000000114e3b497 JavaScriptCore`WTF::AutomaticThread::start(this=0x000000012fff8d38)::$_0::operator()() const at AutomaticThread.cpp:222 frame #7: 0x0000000114e3b0d9 JavaScriptCore`WTF::Function<void ()>::CallableWrapper<WTF::AutomaticThread::start(this=0x000000012fff8d30)::$_0>::call() at Function.h:101 frame #8: 0x0000000114e4b9db JavaScriptCore`WTF::Function<void ()>::operator(this=0x00007000034b6ec0)() const at Function.h:56 frame #9: 0x0000000114ecee9f JavaScriptCore`WTF::Thread::entryPoint(newThreadContext=0x000000012fff3050) at Threading.cpp:136 frame #10: 0x0000000114ed4815 JavaScriptCore`WTF::wtfThreadEntryPoint(context=0x000000012fff3050) at ThreadingPthreads.cpp:223 frame #11: 0x0000000104b0f665 libsystem_pthread.dylib`_pthread_body + 340 frame #12: 0x0000000104b0f511 libsystem_pthread.dylib`_pthread_start + 377 frame #13: 0x0000000104b0ebfd libsystem_pthread.dylib`thread_start + 13
Simon Fraser (smfr)
Comment 8 2018-06-02 13:42:16 PDT
Scratch buffers can be created off the main thread: 2 0x11629d5e9 JSC::VM::scratchBufferForSize(unsigned long) 3 0x1156d7c10 JSC::DFG::JITCompiler::makeCatchOSREntryBuffer() 4 0x1156d83f5 JSC::DFG::JITCompiler::compileFunction() 5 0x1158054bd JSC::DFG::Plan::compileInThreadImpl() 6 0x115802b12 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) 7 0x115a6d17c JSC::DFG::Worklist::ThreadBody::work() 8 0x114e3b3b7 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const 9 0x114e3aff9 WTF::Function<void ()>::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0>::call() 10 0x114e4b8fb WTF::Function<void ()>::operator()() const 11 0x114ecedbf WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) 12 0x114ed4735 WTF::wtfThreadEntryPoint(void*)
Simon Fraser (smfr)
Comment 9 2018-06-02 13:52:18 PDT
Also ScratchBuffer::dataBuffer() can be called off the main thread: 2 0x11573e1e5 JSC::ScratchBuffer::dataBuffer() 3 0x1158bfa56 JSC::DFG::SpeculativeJIT::compileExtractCatchLocal(JSC::DFG::Node*) 4 0x11586a168 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*) 5 0x11585cbba JSC::DFG::SpeculativeJIT::compileCurrentBlock() 6 0x11586b7a3 JSC::DFG::SpeculativeJIT::compile() 7 0x1156d3df7 JSC::DFG::JITCompiler::compileBody() 8 0x1156d88c5 JSC::DFG::JITCompiler::compileFunction() 9 0x1158054bd JSC::DFG::Plan::compileInThreadImpl() 10 0x115802b12 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) 11 0x115a6d17c JSC::DFG::Worklist::ThreadBody::work() 12 0x114e3b377 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const 13 0x114e3afb9 WTF::Function<void ()>::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0>::call() 14 0x114e4b8bb WTF::Function<void ()>::operator()() const 15 0x114eced7f WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
Simon Fraser (smfr)
Comment 10 2018-06-02 14:23:49 PDT
A hack to zero out all scratch buffers on full GC fixes this bug.
Yusuke Suzuki
Comment 11 2018-06-02 15:41:27 PDT
The problem here is that catchOSREntryBuffer's active length is set in non-JIT code, but nobody sets 0 after using it.
Yusuke Suzuki
Comment 12 2018-06-02 15:42:03 PDT
(In reply to Simon Fraser (smfr) from comment #9) > Also ScratchBuffer::dataBuffer() can be called off the main thread: > > 2 0x11573e1e5 JSC::ScratchBuffer::dataBuffer() > 3 0x1158bfa56 > JSC::DFG::SpeculativeJIT::compileExtractCatchLocal(JSC::DFG::Node*) > 4 0x11586a168 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*) > 5 0x11585cbba JSC::DFG::SpeculativeJIT::compileCurrentBlock() > 6 0x11586b7a3 JSC::DFG::SpeculativeJIT::compile() > 7 0x1156d3df7 JSC::DFG::JITCompiler::compileBody() > 8 0x1156d88c5 JSC::DFG::JITCompiler::compileFunction() > 9 0x1158054bd JSC::DFG::Plan::compileInThreadImpl() > 10 0x115802b12 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*) > 11 0x115a6d17c JSC::DFG::Worklist::ThreadBody::work() > 12 0x114e3b377 WTF::AutomaticThread::start(WTF::AbstractLocker > const&)::$_0::operator()() const > 13 0x114e3afb9 WTF::Function<void > ()>::CallableWrapper<WTF::AutomaticThread::start(WTF::AbstractLocker > const&)::$_0>::call() > 14 0x114e4b8bb WTF::Function<void ()>::operator()() const > 15 0x114eced7f WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) Nice catch!
Yusuke Suzuki
Comment 13 2018-06-02 16:39:57 PDT
Created attachment 341855 [details] Patch
Simon Fraser (smfr)
Comment 14 2018-06-03 08:54:11 PDT
I can confirm that this patch fixes the bug.
Keith Miller
Comment 15 2018-06-03 20:27:01 PDT
Comment on attachment 341855 [details] Patch r=me.
Yusuke Suzuki
Comment 16 2018-06-03 20:46:48 PDT
Comment on attachment 341855 [details] Patch Thanks!
WebKit Commit Bot
Comment 17 2018-06-03 21:13:48 PDT
Comment on attachment 341855 [details] Patch Clearing flags on attachment: 341855 Committed r232461: <https://trac.webkit.org/changeset/232461>
WebKit Commit Bot
Comment 18 2018-06-03 21:13:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.