Bug 298798
| Summary: | Make VM::setHasTerminationRequest() thread-safe. | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> |
| Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Mark Lam
VM::setHasTerminationRequest() should be thread-safe but is not because it sets a bit on the VM::m_entryScopeServices OptionSet, which is not thread-safe.
For the record, this is how the concurrent access works:
1. VM::m_entryScopeServices is normally set by the mutator thread running that VM.
2. However, the Worker.terminate() API can be used by the main thread to terminate worker threads.
Worker.terminate() calls ...
WorkerMessagingProxy::terminateWorkerGlobalScope(), which calls ...
WorkerOrWorkletThread::stop(), which calls ...
WorkerOrWorkletScriptController::scheduleExecutionTermination(), which calls ...
VM::notifyNeedTermination(), which calls ...
VM::setHasTerminationRequest(), which calls ...
VM::requestEntryScopeService(), and ...
VM::requestEntryScopeService() does bitwise OR of the EntryScopeService::ResetTerminationRequest bit into the worker thread's VM::m_entryScopeServices. This is not thread safe because it may collide with the worker thread writing values into its VM::m_entryScopeServices.
We can fix this by moving the EntryScopeService::ResetTerminationRequest bit into a separate byte that is safe to write to concurrently.
In practice, the bug never manifests because it is extremely rare for VM::m_entryScopeServices to ever be written to, and the impact is benign.
Regardless, we should fix this.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/160494627>
Mark Lam
Pull request: https://github.com/WebKit/WebKit/pull/50684
EWS
Committed 300049@main (9ea05fa06f20): <https://commits.webkit.org/300049@main>
Reviewed commits have been landed. Closing PR #50684 and removing active labels.