[Proxying] Send messages via in-memory mailbox queues by tlively · Pull Request #18852 · emscripten-core/emscripten

tlively · 2023-02-25T05:26:59Z

Threads were previously notified of new work via postMessage messages that
carried pointers to the task queues to execute. There was no way to synchronously
pump or inspect these pending messages however, and there is no central registry
of all task queues for a thread, so this mechanism afforded no way to discover
or cancel pending work when a thread dies.

In preparation for implementing work cancellation, move the pending messages
into userspace by giving each thread a "mailbox", which is an em_task_queue in
the pthread struct. Instead of using postMessage, proxying queues now use the
thread mailbox API to notify threads of new work.

Internally, thread mailboxes still use postMessage to schedule work to be
executed when a thread returns to its event loop. Since the only task queues
involved in postMessages are now at known locations relative to the pthread
struct, there is no longer any need to store pointers to them in the postMessage
messages themselves. Removing these pointers works around tricky notification
and lifetime management edge cases that would have caused problems such as
dropped work or use-after-free bugs in future PRs.

When a thread dies because it exits or is canceled, it "closes" its mailbox by
decrementing a refcount and waiting to observe a refcount of 0. At this point,
the thread mailbox API ensures that no new messages will be enqueued on the
mailbox. Because the postMessage messages no longer contain task queue pointers,
it is safe to destroy the mailbox immediately after it is closed.

A user-visible behavior change this introduces is that proxied work is more
frequently completed before a thread's main function begins running, since it
no longer gets ordered behind the run message in the JS postMessage queue. A
few tests are updated accordingly.

tlively · 2023-02-25T05:27:11Z

Current dependencies on/for this PR:

main
- PR [Proxying][NFC] Move deferred cleanup logic to em_task_queue #18851
  - PR [Proxying] Send messages via in-memory mailbox queues #18852 👈
    - PR [Proxying] Cancel proxied work when the worker thread dies #18741
      - PR [Proxying] Update proxying with callback to support cancellation #18776
        
        PR [Proxying] Add emscripten_proxy_callback_with_ctx #18810
        
        PR [Proxying] Add proxying functions that return promises #18825
        
        PR [Proxying] Use Atomics.waitAsync to receive proxying notifications #18861
        
        PR Remove unnecessary EXIT_RUNTIME setting from proxying tests #18873

This comment was auto-generated by Graphite.

tlively · 2023-02-28T15:24:43Z

The asan failures (but not other flakes) on test_pthread_dlopen_many seem to be resolved by #18776, so I don't think they're worth investigating here.

sbc100 · 2023-02-28T22:47:03Z

+    // Wait if possible and otherwise spin.
+    if (_emscripten_thread_supports_atomics_wait() &&
+        __builtin_wasm_memory_atomic_wait32(
+          (int*)&thread->mailbox_refcount, count, -1) == 0) {


Can we no use something slightly higher level here such as emscripten_futex_wait?

Yes, I suppose so. And there's not a great reason not to do so, even though I liked programming so close to the (virtual) metal.

I think it would be good to use the emscripten_futex abstraction if we can.

If nothing else I think its nice to limit the places we use __builtin_wasm_memory_atomic_wait32 withing the emscripten codebase.

sbc100 · 2023-02-28T22:48:50Z

+  // can be sure cleanup has finished first.
+
+  // clang-format off
+  EM_ASM({setTimeout(() => Atomics.store(HEAP32, $0 >> 2, 1))}, flag);


Does this need to be EM_ASM or can you just use emscripten_set_timeout?

It needs to be an EM_ASM because we need to avoid callUserCallback. This code will run after the thread runtime has exited.

I see, maybe a comment then?

Threads were previously notified of new work via postMessage messages that carried pointers to the task queues to execute. There was no way to synchronously pump or inspect these pending messages however, and there is no central registry of all task queues for a thread, so this mechanism afforded no way to discover or cancel pending work when a thread dies. In preparation for implementing work cancellation, move the pending messages into userspace by giving each thread a "mailbox", which is an `em_task_queue` in the pthread struct. Instead of using `postMessage`, proxying queues now use the thread mailbox API to notify threads of new work. Internally, thread mailboxes still use postMessage to schedule work to be executed when a thread returns to its event loop. Since the only task queues involved in postMessages are now at known locations relative to the pthread struct, there is no longer any need to store pointers to them in the postMessage messages themselves. Removing these pointers works around tricky notification and lifetime management edge cases that would have caused problems such as dropped work or use-after-free bugs in future PRs. When a thread dies because it exits or is canceled, it "closes" its mailbox by decrementing a refcount and waiting to observe a refcount of 0. At this point, the thread mailbox API ensures that no new messages will be enqueued on the mailbox. Because the postMessage messages no longer contain task queue pointers, it is safe to destroy the mailbox immediately after it is closed. A user-visible behavior change this introduces is that proxied work is more frequently completed _before_ a thread's main function begins running, since it no longer gets ordered behind the `run` message in the JS postMessage queue. A few tests are updated accordingly.

…e#18852) Threads were previously notified of new work via postMessage messages that carried pointers to the task queues to execute. There was no way to synchronously pump or inspect these pending messages however, and there is no central registry of all task queues for a thread, so this mechanism afforded no way to discover or cancel pending work when a thread dies. In preparation for implementing work cancellation, move the pending messages into userspace by giving each thread a "mailbox", which is an `em_task_queue` in the pthread struct. Instead of using `postMessage`, proxying queues now use the thread mailbox API to notify threads of new work. Internally, thread mailboxes still use postMessage to schedule work to be executed when a thread returns to its event loop. Since the only task queues involved in postMessages are now at known locations relative to the pthread struct, there is no longer any need to store pointers to them in the postMessage messages themselves. Removing these pointers works around tricky notification and lifetime management edge cases that would have caused problems such as dropped work or use-after-free bugs in future PRs. When a thread dies because it exits or is canceled, it "closes" its mailbox by decrementing a refcount and waiting to observe a refcount of 0. At this point, the thread mailbox API ensures that no new messages will be enqueued on the mailbox. Because the postMessage messages no longer contain task queue pointers, it is safe to destroy the mailbox immediately after it is closed.

This function has not existing since emscripten-core#18852.

This function has not existed since #18852.

The `checkMailbox` callback can occur after the thread has terminated. In this case calling into native code can trigger the `makeAbortWrapper` wrapper that is put around each native function which then results in a "program has already aborted!" error being thrown. Once solution to this is to make sure that the function which are called do not have `makeAbortWrapper` applied to them. This was the technique I used in emscripten-core#18754, but the list of functions became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852. A better solution is to wrap to whole function in callUserCallback, which takes case of checking if the runtime is alive before calling into native code. Fixes: emscripten-core#20067

The `checkMailbox` callback can occur after the thread has terminated. In this case calling into native code can trigger the `makeAbortWrapper` wrapper that is put around each native function which then results in a "program has already aborted!" error being thrown. Once solution to this is to make sure that the function which are called do not have `makeAbortWrapper` applied to them. This was the technique I used in #18754, but the list of functions became stale when emscripten_proxy_execute_task_queue was removed in #18852. A better solution is to wrap to whole function in callUserCallback, which takes care of checking if the runtime is alive before calling into native code. Fixes: #20067

…pten-core#25064) This function has not existed since emscripten-core#18852.

The `checkMailbox` callback can occur after the thread has terminated. In this case calling into native code can trigger the `makeAbortWrapper` wrapper that is put around each native function which then results in a "program has already aborted!" error being thrown. Once solution to this is to make sure that the function which are called do not have `makeAbortWrapper` applied to them. This was the technique I used in emscripten-core#18754, but the list of functions became stale when emscripten_proxy_execute_task_queue was removed in emscripten-core#18852. A better solution is to wrap to whole function in callUserCallback, which takes care of checking if the runtime is alive before calling into native code. Fixes: emscripten-core#20067

tlively requested a review from sbc100 February 25, 2023 05:27

This was referenced Feb 25, 2023

[Proxying] Add emscripten_proxy_callback_with_ctx #18810

Merged

[Proxying] Update proxying with callback to support cancellation #18776

Merged

[Proxying] Cancel proxied work when the worker thread dies #18741

Merged

This was referenced Feb 25, 2023

[Proxying][NFC] Move deferred cleanup logic to em_task_queue #18851

Merged

[Proxying] Add proxying functions that return promises #18825

Merged

tlively force-pushed the proxying-mailbox branch 2 times, most recently from 73023ce to 6ba6e6d Compare February 25, 2023 06:43

tlively mentioned this pull request Feb 27, 2023

[Proxying] Use Atomics.waitAsync to receive proxying notifications #18861

Merged

tlively mentioned this pull request Feb 28, 2023

Remove unnecessary EXIT_RUNTIME setting from proxying tests #18873

Merged

sbc100 approved these changes Feb 28, 2023

View reviewed changes

Base automatically changed from proxying-zombie-tasks to main February 28, 2023 23:25

tlively added 4 commits February 28, 2023 15:59

Remove mailbox execution before thread start

c0eee50

futex instead of builtins

b312d8a

skip asan dlopen test

bc9b5ad

tlively force-pushed the proxying-mailbox branch from 2f288d3 to bc9b5ad Compare March 1, 2023 01:09

tlively merged commit 3442cda into main Mar 1, 2023

tlively deleted the proxying-mailbox branch March 1, 2023 16:30

dschuff mentioned this pull request Mar 2, 2023

test_pthread_dlopen_many is very flaky #18887

Open

sbc100 added a commit to sbc100/emscripten that referenced this pull request Aug 26, 2025

Remove references to emscripten_proxy_execute_task_queue. NFC

e4ae96a

This function has not existing since emscripten-core#18852.

sbc100 mentioned this pull request Aug 26, 2025

Remove references to emscripten_proxy_execute_task_queue. NFC #25064

Merged

sbc100 added a commit that referenced this pull request Aug 27, 2025

Remove references to emscripten_proxy_execute_task_queue. NFC (#25064)

43b8d7c

This function has not existed since #18852.

sbc100 mentioned this pull request Aug 27, 2025

Fix race condition in checkMailbox #25073

Merged

inolen pushed a commit to inolen/emscripten that referenced this pull request Feb 13, 2026

Remove references to emscripten_proxy_execute_task_queue. NFC (emscri…

f5ba485

…pten-core#25064) This function has not existed since emscripten-core#18852.

Navigation Menu

Uh oh!

[Proxying] Send messages via in-memory mailbox queues - #18852

[Proxying] Send messages via in-memory mailbox queues#18852
tlively merged 4 commits into
mainfrom
proxying-mailbox

tlively commented Feb 25, 2023

Uh oh!

tlively commented Feb 25, 2023 •

edited

Loading

Uh oh!

tlively commented Feb 28, 2023

Uh oh!

sbc100 Feb 28, 2023

Uh oh!

tlively Feb 28, 2023

Uh oh!

sbc100 Feb 28, 2023

Uh oh!

sbc100 Feb 28, 2023

Uh oh!

tlively Feb 28, 2023

Uh oh!

sbc100 Feb 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tlively commented Feb 25, 2023

Uh oh!

tlively commented Feb 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlively commented Feb 28, 2023

Uh oh!

sbc100 Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

tlively Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

sbc100 Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

sbc100 Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

tlively Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

sbc100 Feb 28, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tlively commented Feb 25, 2023 •

edited

Loading