nodejs / help

:sparkles: Need help with Node.js? File an Issue here. :rocket:
1.47k stars 280 forks source link

With a SharedArrayBuffer, is TypedArray.set([...values],index) thread-safe from the point of view of reading threads? #4488

Open nerdgod opened 8 hours ago

nerdgod commented 8 hours ago

Node.js Version

v22.9.0

NPM Version

v10.8.3

Operating System

Windows

Subsystem

worker_threads

Description

Let's say I have a SharedArrayBuffer sab, and various threads have access to it, but only one thread writes to it.

Reader threads map sab to a Uint8Array arr, and they call Atomics.wait(arr,0,0), and wait for the writer to set that flag, before reading from arr[1] onward.

The writer, using its own Uint8Array arr0 mapped to sab, calls

arr0.set([...possibly-many-pages-of-data],1);
Atomics.store(arr0,0,1);
Atomics.notify(arr0,0)

The question is: does Atomics.store() then ensure that arr0.set is a complete write and all threads reading from ANY part of sab (regardless of which portion a given TypedArray may be mapped to, upon which Atomics.notify() was called) have synchronized data once their wait returns ok?

Minimal Reproduction

No response

Output

No response

Before You Submit

nerdgod commented 6 hours ago

In short: Which of the following is true (Assuming only one writing thread, 'main', which uses Atomics.notify() to inform waiting threads that there is an update to the contents of a shared SharedArrayBuffer sab)?:

  1. Everything that the main thread writes to sab is immediately accessible to reader threads once their .wait() returns 'ok'. Not just the value at their waiting index.
  2. Only those things the main thread writes to sab using Atomics.store() are immediately accessible to the reader once their wait() returns 'ok'
  3. Same as (2), but the reader thread must use Atomics.load() to read the store()'d data, for the data to be coherent.
nerdgod commented 2 hours ago

the tc39 reference suggests that TypedArray.set() may be unordered, so I worry that unless I use Atomics, locks may be released before all the pages affected by the set() get written. The reference also says implementers should ensure that "Atomic operations are never rearranged with each other or with non-atomic operations.", which implies I should be able to assume (1), but I can't discern if that's true in practice.

RedYetiDev commented 1 hour ago

I'm not an expert on the specifics of these classes, but whatever the TC39 says should be the case for the project. If you have evidence that suggests otherwise, that's probably a bug.

If this isn't the answer you're looking for, no worries, and someone else better informed than I may have a better response.