[webkit-changes] [WebKit/WebKit] bb3be5: [JSC] Fix race condition in Atomics.{wait, waitAsync}

jjgriego noreply at github.com
Tue Dec 6 12:31:34 PST 2022


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bb3be5c8f260f4dbdece34885f9de06dd5254b6d
      https://github.com/WebKit/WebKit/commit/bb3be5c8f260f4dbdece34885f9de06dd5254b6d
  Author: Joseph Griego <jgriego at igalia.com>
  Date:   2022-12-06 (Tue, 06 Dec 2022)

  Changed paths:
    M JSTests/stress/waitasync-waiter-list-order.js
    M Source/JavaScriptCore/runtime/AtomicsObject.cpp
    M Source/JavaScriptCore/runtime/WaiterListManager.cpp
    M Source/JavaScriptCore/runtime/WaiterListManager.h

  Log Message:
  -----------
  [JSC] Fix race condition in Atomics.{wait,waitAsync}
https://bugs.webkit.org/show_bug.cgi?id=248631

Reviewed by Yusuke Suzuki.

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 `Atomics.wait` and `Atomics.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 delaying the atomic read until the lock on the list of waiters
is held

[1] https://tc39.es/ecma262/multipage/structured-data.html#sec-atomics.wait

* Source/JavaScriptCore/runtime/AtomicsObject.cpp:
(JSC::atomicsWaitImpl):
* Source/JavaScriptCore/runtime/WaiterListManager.cpp:
(JSC::WaiterListManager::waitImpl):
(JSC::WaiterListManager::wait):
(JSC::WaiterListManager::timeoutAsyncWaiter):
(JSC::WaiterListManager::addAsyncWaiter): Deleted.
* Source/JavaScriptCore/runtime/WaiterListManager.h:

Canonical link: https://commits.webkit.org/257423@main




More information about the webkit-changes mailing list