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

Description Simon Fraser (smfr) 2018-06-01 21:29:38 PDT 
After running LayoutTests/fast/css/parsing-css-matches-7.html the Document never goes away.
Comment 1 Simon Fraser (smfr) 2018-06-01 22:36:17 PDT 
Options::useConcurrentGC() = false does NOT fix the bug.
Options::useJIT() = false DOES fix the bug.
Comment 2 Simon Fraser (smfr) 2018-06-01 22:37:27 PDT 
Oddly almost any change simplify the test makes the bug go away.
Comment 3 Simon Fraser (smfr) 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).
Comment 4 Radar WebKit Bug Importer 2018-06-01 23:12:46 PDT 
<rdar://problem/40744972>
Comment 5 Simon Fraser (smfr) 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
Comment 6 Simon Fraser (smfr) 2018-06-02 12:33:43 PDT 
When the DFGJIT is enabled, the JSDOMWindow is treated as a conservative root via gatherScratchBufferRoots().
Comment 7 Simon Fraser (smfr) 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
Comment 8 Simon Fraser (smfr) 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*)
Comment 9 Simon Fraser (smfr) 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*)
Comment 10 Simon Fraser (smfr) 2018-06-02 14:23:49 PDT 
A hack to zero out all scratch buffers on full GC fixes this bug.
Comment 11 Yusuke Suzuki 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.
Comment 12 Yusuke Suzuki 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!
Comment 13 Yusuke Suzuki 2018-06-02 16:39:57 PDT 
Created attachment 341855 [details]
Patch
Comment 14 Simon Fraser (smfr) 2018-06-03 08:54:11 PDT 
I can confirm that this patch fixes the bug.
Comment 15 Keith Miller 2018-06-03 20:27:01 PDT 
Comment on attachment 341855 [details]
Patch

r=me.
Comment 16 Yusuke Suzuki 2018-06-03 20:46:48 PDT 
Comment on attachment 341855 [details]
Patch

Thanks!
Comment 17 WebKit Commit Bot 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>
Comment 18 WebKit Commit Bot 2018-06-03 21:13:50 PDT 
All reviewed patches have been landed.  Closing bug.