Bug 170751 - We need to destroy worker threads in jsc.cpp
Summary: We need to destroy worker threads in jsc.cpp
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
Keywords: InRadar
Depends on: 170628
  Show dependency treegraph
Reported: 2017-04-11 15:00 PDT by JF Bastien
Modified: 2017-05-18 16:47 PDT (History)
9 users (show)

See Also:

Test causing the failure (3.85 KB, application/x-javascript)
2017-04-11 15:01 PDT, JF Bastien
no flags Details
patch (3.57 KB, patch)
2017-05-18 16:45 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2017-04-11 15:00:50 PDT
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
Comment 1 JF Bastien 2017-04-11 15:01:27 PDT
Created attachment 306859 [details]
Test causing the failure
Comment 2 Saam Barati 2017-04-24 12:51:02 PDT
This looks like it could be a serious bug.
Comment 3 Radar WebKit Bug Importer 2017-04-24 16:43:51 PDT
Comment 4 Saam Barati 2017-05-18 15:51:10 PDT
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.
Comment 5 Saam Barati 2017-05-18 16:45:36 PDT
Created attachment 310574 [details]
Comment 6 Saam Barati 2017-05-18 16:46:12 PDT
Comment on attachment 310574 [details]

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

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

oops, I'll undo
Comment 7 Saam Barati 2017-05-18 16:46:35 PDT
I accidentally undid Pizlo's r+. r=pizlo
Comment 8 Saam Barati 2017-05-18 16:47:44 PDT
landed in: