Bug 171380 – REGRESSION (r215843): ASSERTION FAILED: !m_completionTasks[0].first in JSC::Wasm::Plan::tryRemoveVMAndCancelIfLast(JSC::VM &)

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

[Ryan Haddad]

ASSERTION FAILED: !m_completionTasks[0].first
/Volumes/Data/slave/elcapitan-debug/build/Source/JavaScriptCore/wasm/WasmPlan.cpp(116) : bool JSC::Wasm::Plan::tryRemoveVMAndCancelIfLast(JSC::VM &)
1   0x106aefce0 WTFCrash
2   0x106a05673 JSC::Wasm::Plan::tryRemoveVMAndCancelIfLast(JSC::VM&)
3   0x1059baf44 JSC::Wasm::Worklist::stopAllPlansForVM(JSC::VM&)
4   0x106966183 JSC::VM::~VM()
5   0x106967e65 JSC::VM::~VM()
6   0x1061434e9 WTF::ThreadSafeRefCounted<JSC::VM>::deref() const
7   0x106143494 void WTF::derefIfNotNull<JSC::VM>(JSC::VM*)
8   0x1064eb66b WTF::RefPtr<JSC::VM>::operator=(std::nullptr_t)
9   0x1064fbafa JSC::JSLockHolder::~JSLockHolder()
10  0x1064fbb75 JSC::JSLockHolder::~JSLockHolder()
11  0x1110c345d WebCore::WorkerScriptController::~WorkerScriptController()
12  0x1110c3565 WebCore::WorkerScriptController::~WorkerScriptController()
13  0x1110cc058 WebCore::WorkerGlobalScope::clearScript()
14  0x1110c91d6 WebCore::WorkerThread::stop()::$_0::operator()(WebCore::ScriptExecutionContext&) const::'lambda'(WebCore::ScriptExecutionContext&)::operator()(WebCore::ScriptExecutionContext&) const
15  0x1110c9187 WTF::Function<void (WebCore::ScriptExecutionContext&)>::CallableWrapper<WebCore::WorkerThread::stop()::$_0::operator()(WebCore::ScriptExecutionContext&) const::'lambda'(WebCore::ScriptExecutionContext&)>::call(WebCore::ScriptExecutionContext&)
16  0x10e9acc47 WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const
17  0x10e99c52d WebCore::ScriptExecutionContext::Task::performTask(WebCore::ScriptExecutionContext&)
18  0x1110be6d9 WebCore::WorkerRunLoop::Task::performTask(WebCore::WorkerRunLoop const&, WebCore::WorkerGlobalScope*)
19  0x1110be488 WebCore::WorkerRunLoop::runCleanupTasks(WebCore::WorkerGlobalScope*)
20  0x1110bdb43 WebCore::WorkerRunLoop::run(WebCore::WorkerGlobalScope*)
21  0x1110c81b5 WebCore::WorkerThread::runEventLoop()
22  0x10e905a59 WebCore::DedicatedWorkerThread::runEventLoop()
23  0x1110c7fbd WebCore::WorkerThread::workerThread()
24  0x1110c7a25 WebCore::WorkerThread::workerThreadStart(void*)
25  0x106b74579 WTF::Thread::create(void (*)(void*), void*, char const*)::$_0::operator()() const
26  0x106b7454d void std::__1::__invoke_void_return_wrapper<void>::__call<WTF::Thread::create(void (*)(void*), void*, char const*)::$_0&>(WTF::Thread::create(void (*)(void*), void*, char const*)::$_0&&&)
27  0x106b744ec std::__1::__function::__func<WTF::Thread::create(void (*)(void*), void*, char const*)::$_0, std::__1::allocator<WTF::Thread::create(void (*)(void*), void*, char const*)::$_0>, void ()>::operator()()
28  0x1060168aa std::__1::function<void ()>::operator()() const
29  0x106b72e3e WTF::threadEntryPoint(void*)
30  0x106b74fc6 WTF::wtfThreadEntryPoint(void*)
31  0x7fff8d92099d _pthread_body

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK1%20(Tests)/r215860%20(824)/workers/wasm-long-compile-many-crash-log.txt

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=workers%2Fwasm-long-compile-many.html

[Ryan Haddad]

This is a flaky assertion failure seen with LayoutTest workers/wasm-long-compile-many.html

[Radar WebKit Bug Importer]

<rdar://problem/31865427>

[Ryan Haddad]

The first instance of this test crash on the flakiness dashboard is 4/26/2017 10:32:35 PM @ r215853

[Ryan Haddad]

I think this may be related to https://trac.webkit.org/changeset/215843/webkit

[Ryan Haddad]

(In reply to Ryan Haddad from comment #3)
> The first instance of this test crash on the flakiness dashboard is
> 4/26/2017 10:32:35 PM @ r215853

This change was a follow up to r215843.

[Ryan Haddad]

Keith, do you have time to look at this or should we roll out the change?

[Keith Miller]

(In reply to Ryan Haddad from comment #6)
> Keith, do you have time to look at this or should we roll out the change?

I can look at it.

[Keith Miller]

Created attachment 308449 [details]
Patch

[Build Bot]

Attachment 308449 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmModule.h:61:  'compileSync' is incorrectly named. It should be named 'protector' or 'protectedMemoryMode'.  [readability/naming/protected] [4]
ERROR: Source/JavaScriptCore/wasm/WasmModule.h:68:  'getOrCreateCodeBlock' is incorrectly named. It should be named 'protector' or 'protectedMemoryMode'.  [readability/naming/protected] [4]
Total errors found: 2 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

(In reply to Build Bot from comment #9)
> Attachment 308449 [details] did not pass style-queue:
> 
> 
> ERROR: Source/JavaScriptCore/wasm/WasmModule.h:61:  'compileSync' is
> incorrectly named. It should be named 'protector' or 'protectedMemoryMode'. 
> [readability/naming/protected] [4]
> ERROR: Source/JavaScriptCore/wasm/WasmModule.h:68:  'getOrCreateCodeBlock'
> is incorrectly named. It should be named 'protector' or
> 'protectedMemoryMode'.  [readability/naming/protected] [4]
> Total errors found: 2 in 14 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

Wat?

[JF Bastien]

Comment on attachment 308449 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:10
> +        we want the main task to not be associated with any VM.

Can you clarify what "some" and "others" cases are?

[Keith Miller]

Created attachment 308452 [details]
Patch for landing

[Saam Barati]

Comment on attachment 308449 [details]
Patch

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

> Source/JavaScriptCore/wasm/WasmPlan.cpp:104
> +    if (!ASSERT_DISABLED) {
> +        // We allow the first completion task to not have a vm.
> +        for (unsigned i = 1; i < m_completionTasks.size(); ++i)
> +            ASSERT(m_completionTasks[i].first);
> +    }

Please add your FIXME here w/ the cleanup bug to make index 0 not magical.

[Keith Miller]

Comment on attachment 308449 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:10
>> +        we want the main task to not be associated with any VM.
> 
> Can you clarify what "some" and "others" cases are?

Validation and BBQ, respectively. I changed the changelog (that sounds deep).

[Keith Miller]

Created attachment 308454 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 308454 [details]
Patch for landing

Clearing flags on attachment: 308454

Committed r215896: <http://trac.webkit.org/changeset/215896>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.