Bug 226096 - Finalize DFG/FTL code refs on the compiler threads
Summary: Finalize DFG/FTL code refs on the compiler threads
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on: 226180
Blocks:
  Show dependency treegraph
 
Reported: 2021-05-21 11:41 PDT by Saam Barati
Modified: 2021-05-25 11:17 PDT (History)
6 users (show)

See Also:


Attachments
WIP (65.34 KB, patch)
2021-05-21 17:06 PDT, Saam Barati
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
WIP (65.92 KB, patch)
2021-05-21 17:38 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
WIP (66.16 KB, patch)
2021-05-21 18:03 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
patch (25.55 KB, patch)
2021-05-24 17:28 PDT, Saam Barati
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (25.55 KB, patch)
2021-05-24 17:46 PDT, Saam Barati
mark.lam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
[fast-cq] patch for landing (23.92 KB, patch)
2021-05-25 11:15 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 Saam Barati 2021-05-21 11:41:15 PDT
...
Comment 1 Saam Barati 2021-05-21 17:06:56 PDT
Created attachment 429369 [details]
WIP
Comment 2 Saam Barati 2021-05-21 17:38:11 PDT
Created attachment 429376 [details]
WIP
Comment 3 Saam Barati 2021-05-21 18:03:18 PDT
Created attachment 429378 [details]
WIP
Comment 4 Saam Barati 2021-05-24 17:28:29 PDT
Created attachment 429599 [details]
patch
Comment 5 Saam Barati 2021-05-24 17:46:09 PDT
Created attachment 429602 [details]
patch
Comment 6 Mark Lam 2021-05-24 19:56:31 PDT
Comment on attachment 429602 [details]
patch

LGTM but the mac-wk1 EWS's failure seems real.  The failure is in the RELEASE_ASSERT in ~LinkBuffer on a worker thread at VM shutdown.  

Thread 77 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x0000000107d106d3 WTFCrashWithInfo(int, char const*, char const*, int) + 19
1   com.apple.JavaScriptCore      	0x00000001073eff0a JSC::LinkBuffer::~LinkBuffer() + 58
2   com.apple.JavaScriptCore      	0x0000000107839e5d JSC::DFG::JITFinalizer::~JITFinalizer() + 45
3   com.apple.JavaScriptCore      	0x0000000107906b18 JSC::DFG::Plan::~Plan() + 312
4   com.apple.JavaScriptCore      	0x000000010795e264 JSC::DFG::Worklist::removeAllReadyPlansForVM(JSC::VM&) + 116
5   com.apple.JavaScriptCore      	0x0000000107f961e7 JSC::VM::~VM() + 279
6   com.apple.JavaScriptCore      	0x0000000107e3a7d7 JSC::JSLockHolder::~JSLockHolder() + 119
7   com.apple.WebCore             	0x000000010e069754 WebCore::WorkerOrWorkletScriptController::~WorkerOrWorkletScriptController() + 212
8   com.apple.WebCore             	0x000000010e071195 WTF::Detail::CallableWrapper<WebCore::WorkerOrWorkletThread::stop(WTF::Function<void ()>&&)::$_34::operator()(WebCore::ScriptExecutionContext&) const::'lambda'(WebCore::ScriptExecutionContext&), void, WebCore::ScriptExecutionContext&>::call(WebCore::ScriptExecutionContext&) + 37
9   com.apple.WebCore             	0x000000010e071816 WebCore::WorkerRunLoop::runCleanupTasks(WebCore::WorkerOrWorkletGlobalScope*) + 262
10  com.apple.WebCore             	0x000000010e0713bf WebCore::WorkerRunLoop::run(WebCore::WorkerOrWorkletGlobalScope*) + 111
11  com.apple.WebCore             	0x000000010e06ca30 WebCore::WorkerOrWorkletThread::workerOrWorkletThread() + 768
12  com.apple.JavaScriptCore      	0x00000001070739ec WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 124
13  com.apple.JavaScriptCore      	0x0000000107076259 WTF::wtfThreadEntryPoint(void*) + 9
14  libsystem_pthread.dylib       	0x00007fff72768109 _pthread_start + 148
15  libsystem_pthread.dylib       	0x00007fff72763b8b thread_start + 15

Looks like possibly the plan wasn't cancelled, or not cancelled in time?
Comment 7 Saam Barati 2021-05-24 20:31:03 PDT
Comment on attachment 429602 [details]
patch

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

> Source/JavaScriptCore/assembler/LinkBuffer.cpp:49
> +        RELEASE_ASSERT(m_mainThreadFinalizationTasks.isEmpty());

I’ll remove this RELEASE_ASSERT. It’s taking a lot of plumbing to make it work. Which is usually a bad reason to keep an assert
Comment 8 Mark Lam 2021-05-24 20:52:07 PDT
Comment on attachment 429602 [details]
patch

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

r=me

>> Source/JavaScriptCore/assembler/LinkBuffer.cpp:49
>> +        RELEASE_ASSERT(m_mainThreadFinalizationTasks.isEmpty());
> 
> I’ll remove this RELEASE_ASSERT. It’s taking a lot of plumbing to make it work. Which is usually a bad reason to keep an assert

I'm fine with removing it.  The failure was at VM destruction, and m_mainThreadFinalizationTasks will get cleaned out anyway.
Comment 9 Saam Barati 2021-05-25 11:15:32 PDT
Created attachment 429668 [details]
[fast-cq] patch for landing
Comment 10 EWS 2021-05-25 11:16:55 PDT
Committed r278030 (238127@main): <https://commits.webkit.org/238127@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429668 [details].
Comment 11 Radar WebKit Bug Importer 2021-05-25 11:17:20 PDT
<rdar://problem/78466944>