Bug 248631
| Summary: | [JSC] Race condition in Atomic.wait, Atomic.waitAsync | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Joseph Griego <joseph.j.griego> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | webkit-bug-importer |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Joseph Griego
At the moment, this is manifesting as intermittent timeouts in the arm32 linux test runner, for example: https://build.webkit.org/#/builders/24/builds/17430
After investigation, this is the test `stress/waitasync-waiter-list-order.js` hanging; this manifests as some of the workers hanging on the initial `Atomic.wait` call in that test--using a debugger to examine the memory being waited on reveals that they should all have woken up and moved on.
The implementation of both `Atomic.wait` and `Atomic.waitAsync` begin by atomically reading from the array and comparing the result to the expected value, then obtaining a lock on the list of waiters and proceeding from there--this is contrary to the spec [1], which requires that the atomic read happens while the list of waiters is exclusively locked and allows a race where the following interleaving results in A hanging indefinitely if no other calls to Atomic.notify on that address occur:
thread A: begin Atomic.wait(buffer, idx, expected_value)
thread A: read buffer[idx], get expected_value
thread B: perform Atomic.store(buffer, idx, unexpected_value)
thread B: perform Atomic.notify(buffer, idx) [nothing is notified since the list of waiters is empty]
thread A: lock list of waiters, wait on condition variable, hang waiting for next notify
---
This is fixed by performing the atomic read after the list of waiters is locked
Patch forthcoming.
[1] https://tc39.es/ecma262/multipage/structured-data.html#sec-atomics.wait
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Joseph Griego
Pull request: https://github.com/WebKit/WebKit/pull/7048
EWS
Committed 257423@main (bb3be5c8f260): <https://commits.webkit.org/257423@main>
Reviewed commits have been landed. Closing PR #7048 and removing active labels.