denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
95.93k stars 5.3k forks source link

Atomics.waitAsync bug #14786

Open Eyal-Shalev opened 2 years ago

Eyal-Shalev commented 2 years ago

According to the Atomics.waitAsync documentation, it:

verifies that a given position in an Int32Array still contains a given value and if so sleeps, awaiting a wakeup or a timeout.

However, in my testing it seems like the waitAsync doesn't wake (even after calling Atomics.notify).

Moreover, I think that the event loop might be stuck (setTimeout didn't work and the test gets stuck) but I don't have any way to verify that.

Below are the POC files I used to verify that there is a problem.

poc_test.ts

Deno.test(async function poc() {
  const ia = new Int32Array(
    new SharedArrayBuffer(Int32Array.BYTES_PER_ELEMENT),
  );

  const p = Promise.allSettled(
    Array(2).fill(0).map((_, i) => {
      const name = `poc_worker#${i}`;
      const w = new Worker(
        new URL("testdata/poc_worker.ts", import.meta.url),
        {
          type: "module",
          name,
        },
      );
      return new Promise<void>((res, rej) => {
        w.onmessage = (ev) => {
          if (ev.data.ok) res(ev.data);
          else rej(ev.data);
        };
        setTimeout(w.postMessage.bind(w), undefined, ia);
      }).finally(w.terminate.bind(w));
    }),
  );

  console.log(await p);
});

testdata/poc_worker.ts

export const sleep = (duration: number) => {
  return new Promise<void>((res) => {
    setTimeout(() => res(), duration);
  });
};

self.onmessage = async (ev) => {
  const ia = ev.data;
  console.log(self.name, ia);
  try {
    const actual = Atomics.compareExchange(ia, 0, 0, 1);

    if (actual === 0) {
      await sleep(0);
      const notified = Atomics.notify(ia, 0);
      console.log(self.name, { actual, notified });
      self.postMessage({ ok: true });
    } else {
      // @ts-expect-error see https://github.com/microsoft/TypeScript/issues/49198
      let { async, value } = Atomics.waitAsync(ia, 0, 1);
      console.log(self.name, { actual, async, value });
      value = await value;
      self.postMessage({ ok: true });
    }
  } catch (err) {
    self.postMessage({ ok: false, err });
  }
};

output

------- output -------
poc_worker#1 Int32Array(1) [ 0 ]
poc_worker#1 { actual: 0, notified: 0 }
poc_worker#0 Int32Array(1) [ 1 ]
poc_worker#0 { actual: 1, async: true, value: Promise { <pending> } }

My Setup WSL2 - Ubuntu

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 20.04.4 LTS
Release:        20.04
Codename:       focal

$ deno --version
deno 1.22.1 (release, x86_64-unknown-linux-gnu)
v8 10.3.174.6
typescript 4.6.2
stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

Eyal-Shalev commented 1 year ago

Note that this bug was never fixed.

Eyal-Shalev commented 1 year ago

Wanted to see if I can help find the bug, but I can't even find the place where Atomics.waitAsync is implemented.

andreubotella commented 1 year ago

Wanted to see if I can help find the bug, but I can't even find the place where Atomics.waitAsync is implemented.

Deno doesn't implement Atomics.waitAsync, that's implemented in V8, the JS engine that Deno (as well as Node.js and Chrome) use. Assuming this is not a V8 bug, it's almost certainly related to Deno's event loop code not knowing about Atomics.waitAsync and only knowing about "ops" (Deno's word for Rust functions that are passed into deno_core so they get exposed into the Javascript VM). The relevant code is in JsRuntime::poll_event_loop (or perhaps elsewhere in JsRuntime), but I doubt it would be easy to fix.

aapoalas commented 1 year ago

It's implemented by V8 itself. There's probably some API in V8 to either hook into these APIs or query the number or active timers like this.

Eyal-Shalev commented 1 year ago

I see that there is a test for Atomics.waitAsync in runtime.rs but it doesn't involve a worker.

I tried to create a failing test that uses a Blob Worker, but it complains that Blob is not known - #16747

Do you know how to add the Blob symbol to the runtime for the test?

thread 'runtime::tests::test_pump_message_loop_2' panicked at 'called `Result::unwrap()` on an `Err` value: ReferenceError: Blob is not defined
    at pump_message_loop.js:2:1', core/runtime.rs:3356:10
aapoalas commented 1 year ago

My duplicate issue: https://github.com/denoland/deno/issues/15358

kaffeeumeins commented 2 months ago

Any news on this. I facing also issues with Atomics.waitAsync does not resolve after calling to Atomics.notify.