1942674 - Atomics.wait returning incorrect result

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect, P3)

Description

[Bryan Gillespie]

Attachment: firefox.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36

Steps to reproduce:

I have a simple SharedArrayBuffer setup where web workers call main thread via postMessage and then synchronously block for a response using Atomics.notify and Atomics.wait. Crucially, I have a second Atomics.wait which serves purely as a synchronous sleep mechanism.

//////////////////// WORKER
function workerFn() {
    addEventListener("message", (e) => {
        console.log('Worker received message', e.data);
        let sbuf = {
            sab: e.data,
            vi32: new Int32Array(e.data),
        }

        while (true) {
            postMessage("");
            Atomics.wait(sbuf.vi32, 0, 0); // Wait for expected value to change
            Atomics.store(sbuf.vi32, 0, 0); // Ok, signal is changed, reset expected value to 0 for next iteration

            // Sleep for 1 ms
            let rv = Atomics.wait(sbuf.vi32, 0, 0, 1);
            if (rv !== "timed-out") {
                const signal = Atomics.load(sbuf.vi32, 0);
                throw new Error("How is this possible? Impossible return value:" + rv + " signal:" + signal);
            }
        }
    });
}

//////////////////// MAIN THREAD
let processedMessages = 0;
function onWorkerMessage(workerName, data) {
    if (++processedMessages % 10000 === 0) console.log('Processed', processedMessages, 'messages');

    // Notify worker to wake up and sleep for 1 ms
    const sbuf = sbufs[workerName];
    Atomics.store(sbuf.vi32, 0, 1);
    Atomics.notify(sbuf.vi32, 0);
}

//////////////////// INIT
let sbufs = {}
window.sbufs = sbufs
for (let i = 0; i < 20; i++) {
    console.log('Starting worker', i)

    // Get convert worker function to blob
    let wf = workerFn.toString();
    wf = wf.substring(wf.indexOf('{') + 1, wf.lastIndexOf('}')) 
    const blob = new Blob([wf], { type: 'application/javascript' })

    const worker = new Worker(URL.createObjectURL(blob), { name: `worker${i}` })
    worker.onmessage = e => onWorkerMessage(`worker${i}`, e.data)

    const sab = new SharedArrayBuffer(4) // [0:4 Atomics.wait signal]
    const vi32 = new Int32Array(sab); // View needed for Atomics.wait
    sbufs[`worker${i}`] = { sab, vi32 };
    worker.postMessage(sab);
}

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:134.0) Gecko/20100101 Firefox/134.0

Note this is also reproducible in Chrome (https://issues.chromium.org/issues/391092762)

Actual results:

The second Atomics.wait serving as a synchronous sleep is not returning timed-out every time, but also ok and (rarely) not-equal. This should not be possible because I set the expected value on the previous line and it should not be possible for the expected value to change again until the next postMessage is called. However, incorrect return values are happening, and the frequency of the incorrect return value seems to scale with the number of web workers. I'm not 100% certain, but this may be indicative of larger issues I'm observing when doing synchronization between many web workers and a main thread (https://stackoverflow.com/q/79370468/2516916).

Expected results:

The Atomics.wait serving as a sleep should always return timed-out.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Workers' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[André Bargull [:anba]]

The main and worker threads can be preempted at any given time, which means the following two executions are possible.

Here's a possible execution where Atomics.wait will return "ok".

Main                                    Worker
                                        postMessage("");
Atomics.store(sbuf.vi32, 0, 1);                                             // Disallow wait
                                        Atomics.wait(sbuf.vi32, 0, 0);      // No wait
                                        Atomics.store(sbuf.vi32, 0, 0);     // Allow wait
                                        Atomics.wait(sbuf.vi32, 0, 0, 1);   // Wait
Atomics.notify(sbuf.vi32, 0);                                               // Wake up worker

And this is a possible execution where Atomics.wait will return "not-equal".

Main                                    Worker
                                        postMessage("");
Atomics.store(sbuf.vi32, 0, 1);
                                        Atomics.wait(sbuf.vi32, 0, 0);      // No wait
                                        Atomics.store(sbuf.vi32, 0, 0);     // Allow wait
                                        Atomics.wait(sbuf.vi32, 0, 0, 1);   // Wait
                                                                            // Timeout!
                                        postMessage("");
                                        Atomics.wait(sbuf.vi32, 0, 0);      // Wait
Atomics.notify(sbuf.vi32, 0);                                               // Wake up worker
                                        Atomics.store(sbuf.vi32, 0, 0);     // Allow wait
Atomics.store(sbuf.vi32, 0, 1);                                             // Disallow wait
                                        Atomics.wait(sbuf.vi32, 0, 0, 1);   // No wait, values not equal.
Atomics.notify(sbuf.vi32, 0);

Comment 3

[Bryan Gillespie]

Yes, I had considered that interleaving previously, but you're saying that doing a notify on a on wait whose value has not changed results in ok?

                                        Atomics.store(sbuf.vi32, 0, 0);     // Allow wait
                                        Atomics.wait(sbuf.vi32, 0, 0, 1);   // Wait
Atomics.notify(sbuf.vi32, 0);                                               // <-- Shouldn't this be a no-op because sbuf.vi32[0] is still 0?

I think maybe I've fundamentally misunderstood the docs:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/wait
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/notify

My understanding of

The Atomics.wait() static method verifies that a shared memory location still contains a given value and if so sleeps, awaiting a wake-up notification or times out

and

A reading thread is sleeping and waiting on location 0 which is expected to be 0. As long as that is true, it will not go on.

Is that Atomics.wait will not move on unless:

• The values weren't equal to start with (resulting in an immediate not-equal)
• The value under watch eventually changed (resulting in ok)
• The wait timed out (resulting in timed-out)

But what you are telling me is at odds with that interpretation of the docs. If that's the case we can go ahead and close this ticket, and I will explore avenues for making the docs more clear.

Comment 4

[Bryan Gillespie]

Ok, I created a PR to update the docs here: https://github.com/mdn/content/pull/37742

In light of this clarification, I confirmed that the following changes make my original example behave as expected:

let rv = Atomics.wait(sbuf.vi32, 1, 0, 1); // Listen on a different index so that it can't be awoken by any interleaving

and

do {
    Atomics.wait(sbuf.vi32, 0, 0); // Wait for expected value to change, even if it is awoken
} while (Atomics.load(sbuf.vi32, 0) === 0)

We can go ahead and close this ticket.

Comment 5

[André Bargull [:anba]]

(In reply to Bryan Gillespie from comment #4)

Ok, I created a PR to update the docs here: https://github.com/mdn/content/pull/37742

Thanks for creating a PR to improve the docs!

We can go ahead and close this ticket.

Alright, I'll close this issue then.

Comment 6

[Bryan Gillespie]

Thank you André for taking the time to correct my incorrect understanding!