Bug 199585 - Make Document::postTask() safe to call from a background thread
Summary: Make Document::postTask() safe to call from a background thread
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 199517
  Show dependency treegraph
 
Reported: 2019-07-08 12:51 PDT by Chris Dumez
Modified: 2019-07-08 19:26 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.80 KB, patch)
2019-07-08 12:55 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-07-08 12:51:55 PDT
Make Document::postTask() safe to call from a background thread.
Comment 1 Chris Dumez 2019-07-08 12:55:09 PDT
Created attachment 373657 [details]
Patch
Comment 2 Chris Dumez 2019-07-08 12:56:16 PDT
Here is an example of legit case where we're calling Document::postTask() from a background thread:
Thread 16 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x000000057173035e WTFCrash + 14 (Assertions.cpp:305)
1   com.apple.WebCore             	0x0000000559f463eb WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x000000055b6e114c WebCore::ContainerNode::WeakValueType* WTF::WeakPtrImpl::get<WebCore::ContainerNode>() + 140 (WeakPtr.h:65)
3   com.apple.WebCore             	0x000000055b6e0f7a WTF::WeakPtrFactory<WebCore::ContainerNode>::createWeakPtr(WebCore::ContainerNode&) const + 138 (WeakPtr.h:142)
4   com.apple.WebCore             	0x000000055b6d70ff WTF::WeakPtr<WebCore::Document> WTF::makeWeakPtr<WebCore::Document>(WebCore::Document&) + 63 (WeakPtr.h:212)
5   com.apple.WebCore             	0x000000055c4a21a0 WebCore::Document::postTask(WebCore::ScriptExecutionContext::Task&&) + 32 (Document.cpp:6121)
6   com.apple.WebCore             	0x000000055de23635 WebCore::WorkerMessagingProxy::reportPendingActivity(bool) + 101 (WorkerMessagingProxy.cpp:283)
7   com.apple.WebCore             	0x000000055de01793 WebCore::DedicatedWorkerThread::runEventLoop() + 83 (DedicatedWorkerThread.cpp:57)
8   com.apple.WebCore             	0x000000055de27710 WebCore::WorkerThread::workerThread() + 1184 (WorkerThread.cpp:206)
9   com.apple.WebCore             	0x000000055de37c58 WebCore::WorkerThread::start(WTF::Function<void (WTF::String const&)>&&)::$_12::operator()() const + 24 (WorkerThread.cpp:150)
10  com.apple.WebCore             	0x000000055de37c19 WTF::Detail::CallableWrapper<WebCore::WorkerThread::start(WTF::Function<void (WTF::String const&)>&&)::$_12, void>::call() + 25 (Function.h:52)
11  com.apple.JavaScriptCore      	0x000000057175ad6a WTF::Function<void ()>::operator()() const + 138 (Function.h:79)
12  com.apple.JavaScriptCore      	0x00000005717f22f0 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 416 (Threading.cpp:149)
13  com.apple.JavaScriptCore      	0x00000005717fa725 WTF::wtfThreadEntryPoint(void*) + 21 (ThreadingPOSIX.cpp:200)
14  libsystem_pthread.dylib       	0x00007fff6dd36daa _pthread_start + 125
15  libsystem_pthread.dylib       	0x00007fff6dd336af thread_start + 15
Comment 3 Alex Christensen 2019-07-08 16:43:25 PDT
Comment on attachment 373657 [details]
Patch

Maybe we should add some assertions in makeWeakPtr to make sure it's not called from the wrong thread anywhere else.
Comment 4 Chris Dumez 2019-07-08 16:55:40 PDT
(In reply to Alex Christensen from comment #3)
> Comment on attachment 373657 [details]
> Patch
> 
> Maybe we should add some assertions in makeWeakPtr to make sure it's not
> called from the wrong thread anywhere else.

Well, this is what I am doing in Bug 199517, and this is how I found this bug (and several others already).
Comment 5 Chris Dumez 2019-07-08 16:56:49 PDT
Comment on attachment 373657 [details]
Patch

Clearing flags on attachment: 373657

Committed r247239: <https://trac.webkit.org/changeset/247239>
Comment 6 Chris Dumez 2019-07-08 16:56:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-07-08 16:59:29 PDT
<rdar://problem/52805445>
Comment 8 Ryosuke Niwa 2019-07-08 19:26:21 PDT
Comment on attachment 373657 [details]
Patch

Nice catch!