Bug 170751 – We need to destroy worker threads in jsc.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[JF Bastien]

A test I'm adding for bug #170628 is tripping an assertion failure in tip-of-tree (without my change, just this test). Older wasm memory code is broken so my repro disables fast memory (after #170628 it'll also repro, even with fast memory).

The crash isn't deterministic, only happen 1/10 times or so.

$ for i in `seq 1 1000`; do (cd ./JSTests/wasm/ && JSC_useWebAssemblyFastMemory=0  ../../current-debug/bin/jsc -m ./function-tests/memory-multiagent.js  ); done
ASSERTION FAILED: mergeSpeculations(type, speculationFromValue(m_value)) == type
/Volumes/dev/wk/OpenSource/Source/JavaScriptCore/dfg/DFGAbstractValue.cpp(510) : void JSC::DFG::AbstractValue::checkConsistency() const
1   0x103e83b7d WTFCrash
2   0x103217f99 JSC::DFG::AbstractValue::checkConsistency() const
3   0x1032b0b22 JSC::DFG::AbstractValue::observeInvalidationPoint()
4   0x1032ada25 JSC::DFG::AbstractValue::observeInvalidationPointFor(JSC::DFG::AbstractValue&)
5   0x1032b0aec void JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::forAllValues<void (JSC::DFG::AbstractValue&)>(unsigned int, void (&)(JSC::DFG::AbstractValue&))::'lambda'(JSC::DFG::NodeFlowProjection)::operator()(JSC::DFG::NodeFlowProjection) const
6   0x1032b0a65 void JSC::DFG::NodeFlowProjection::forEach<void JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::forAllValues<void (JSC::DFG::AbstractValue&)>(unsigned int, void (&)(JSC::DFG::AbstractValue&))::'lambda'(JSC::DFG::NodeFlowProjection)>(JSC::DFG::Node*, void  const(&)(JSC::DFG::AbstractValue&))
7   0x1032ad83b void JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::forAllValues<void (JSC::DFG::AbstractValue&)>(unsigned int, void (&)(JSC::DFG::AbstractValue&))
8   0x1032aa172 JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*)
9   0x1035142f3 JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int)
10  0x103513eda JSC::DFG::SpeculativeJIT::compileCurrentBlock()
11  0x103514953 JSC::DFG::SpeculativeJIT::compile()
12  0x1033e7ec7 JSC::DFG::JITCompiler::compileBody()
13  0x1033ea91e JSC::DFG::JITCompiler::compile()
14  0x1034c1bef JSC::DFG::Plan::compileInThreadImpl(JSC::DFG::LongLivedState&)
15  0x1034bef49 JSC::DFG::Plan::compileInThread(JSC::DFG::LongLivedState&, JSC::DFG::ThreadData*)
16  0x103619ab5 JSC::DFG::Worklist::ThreadBody::work()
17  0x103e88907 WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0::operator()() const
18  0x103e886ad void std::__1::__invoke_void_return_wrapper<void>::__call<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0&>(WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0&&&)
19  0x103e884f9 std::__1::__function::__func<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0, std::__1::allocator<WTF::AutomaticThread::start(WTF::AbstractLocker const&)::$_0>, void ()>::operator()()
20  0x10350acca std::__1::function<void ()>::operator()() const
21  0x103ea73d7 WTF::threadEntryPoint(void*)
22  0x103ef0f81 WTF::wtfThreadEntryPoint(void*)
23  0x7fffa1bc9aab _pthread_body
24  0x7fffa1bc99f7 _pthread_body
25  0x7fffa1bc91fd thread_start
ASSERTION FAILED: mergeSpeculations(type, speculationFromValue(m_value)) == type

[JF Bastien]

Created attachment 306859 [details]
Test causing the failure

[Saam Barati]

This looks like it could be a serious bug.

[Radar WebKit Bug Importer]

<rdar://problem/31800412>

[Saam Barati]

The bug is as follows:
- We're asserting that a string that we observed as being atomic is no longer atomic.
- This happens because we create the $agent thread, start compiling code, then destroy the thread, all before the compilation finishes. This will lead to the thread's atomic string table being destroyed.

We have two options:
1. Wait for compilations to finish for workers
2. Just destroy worker VMs

I'm choosing 2 since this is what WebCore does and it's probably good testing.

We're not going to destroy the VM when it's the main thread VM since the main thread is just going to exit.

[Saam Barati]

Created attachment 310574 [details]
patch

[Saam Barati]

Comment on attachment 310574 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310574&action=review

> Source/JavaScriptCore/jsc.cpp:3855
> +

oops, I'll undo

[Saam Barati]

I accidentally undid Pizlo's r+. r=pizlo

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/217077/webkit