Skip to content

WASMFS Support blocking poll/select - #28205

Open
aaalloc wants to merge 3 commits into
emscripten-core:mainfrom
aaalloc:support_wasmfs_select_poll
Open

aaalloc wants to merge 3 commits into
emscripten-core:mainfrom
aaalloc:support_wasmfs_select_poll

Conversation

@aaalloc

@aaalloc aaalloc commented Nov 7, 2026

Copy link
Copy Markdown

related to #26736

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements blocking support for poll() and select() in WasmFS by introducing wait queues (PollWaiter, WaitQueue, and WaitRegistration). Pipes are updated to utilize these queues to notify waiting threads of state changes, and __syscall_poll is refactored to block when a timeout is specified. Consequently, several blocking select and poll tests have been re-enabled. The feedback suggests optimizing __syscall_poll by calling reserve(nfds) on the files and regs vectors to prevent multiple dynamic allocations and moves of WaitRegistration objects.

Comment thread system/lib/wasmfs/syscalls.cpp Resolved
@aaalloc
aaalloc force-pushed the support_wasmfs_select_poll branch from 9a13164 to 196182f Compare November 7, 2026 21:39
@aaalloc
aaalloc marked this pull request as draft November 8, 2026 09:53
@aaalloc
aaalloc force-pushed the support_wasmfs_select_poll branch from b64f10e to daa6bd7 Compare November 8, 2026 12:12
@aaalloc
aaalloc force-pushed the support_wasmfs_select_poll branch from daa6bd7 to cf84030 Compare November 8, 2026 12:13
@aaalloc
aaalloc marked this pull request as ready for review November 8, 2026 13:05

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements blocking poll() support in WasmFS using wait queues, allowing threads to block until a file descriptor becomes ready. It introduces PollWaiter, WaitQueue, and WaitRegistration classes, integrates them with PipeFile and __syscall_poll, and adds JS library helpers to support asynchronous polling on the main browser thread. The review feedback highlights a critical race condition (lost wakeup) between _wasmfs_poll_wait and _wasmfs_poll_notify when yielding via Asyncify on the main thread. To resolve this, a 'pre-triggered' state should be supported in wasmFSPollWaiters to capture notifications that arrive before the wait is registered, and both functions should declare $wasmFSPollWaiters in their __deps.

Comment thread src/lib/libwasmfs.js
Comment on lines +569 to 597
_wasmfs_poll_wait__async: 'auto',
_wasmfs_poll_wait__sig: 'ipi',
_wasmfs_poll_wait: (key, ms, sync) => {
if (sync) {
return 0;
} else {
return new Promise((resolve) => {
var timer;
if (ms >= 0) {
timer = setTimeout(() => {
delete wasmFSPollWaiters[key];
resolve(0);
}, ms);
}
wasmFSPollWaiters[key] = {resolve, timer};
});
}
},

_wasmfs_poll_notify__sig: 'vp',
_wasmfs_poll_notify: (key) => {
var entry = wasmFSPollWaiters[key];
if (!entry)
return;

delete wasmFSPollWaiters[key];
clearTimeout(entry.timer);
entry.resolve(1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There is a critical race condition (lost wakeup) between _wasmfs_poll_wait and _wasmfs_poll_notify. When waitAsync is called on the main thread, it unlocks the C++ mutex before calling _wasmfs_poll_wait (which is necessary because it yields via Asyncify). If a worker thread calls notifyAll() and proxies _wasmfs_poll_notify during this brief window, _wasmfs_poll_notify will see that wasmFSPollWaiters[key] is not yet registered, and will return immediately without doing anything. Then, _wasmfs_poll_wait will run, register itself, and block indefinitely (or until timeout), missing the notification entirely.

To fix this, we can support a "pre-triggered" state in wasmFSPollWaiters. If _wasmfs_poll_notify is called before _wasmfs_poll_wait is registered, we record that the key was triggered. When _wasmfs_poll_wait is subsequently called, it will immediately consume the trigger and return 1 synchronously, avoiding the race condition and also avoiding an unnecessary Asyncify yield.

Additionally, both _wasmfs_poll_wait and _wasmfs_poll_notify use the $wasmFSPollWaiters library variable, so they must declare it in their __deps to ensure the compiler includes and handles it correctly.

  _wasmfs_poll_wait__deps: ['$wasmFSPollWaiters'],
  _wasmfs_poll_wait__async: 'auto',
  _wasmfs_poll_wait__sig: 'ipi',
  _wasmfs_poll_wait: (key, ms, sync) => {
    if (sync) {
      return 0;
    }
    var entry = wasmFSPollWaiters[key];
    if (entry && entry.triggered) {
      delete wasmFSPollWaiters[key];
      return 1;
    }
    return new Promise((resolve) => {
      var timer;
      if (ms >= 0) {
        timer = setTimeout(() => {
          delete wasmFSPollWaiters[key];
          resolve(0);
        }, ms);
      }
      wasmFSPollWaiters[key] = {resolve, timer};
    });
  },

  _wasmfs_poll_notify__deps: ['$wasmFSPollWaiters'],
  _wasmfs_poll_notify__sig: 'vp',
  _wasmfs_poll_notify: (key) => {
    var entry = wasmFSPollWaiters[key];
    if (!entry) {
      wasmFSPollWaiters[key] = { triggered: true };
      return;
    }
    if (entry.triggered) {
      return;
    }
    delete wasmFSPollWaiters[key];
    clearTimeout(entry.timer);
    if (entry.resolve) {
      entry.resolve(1);
    }
  }

@aaalloc
aaalloc force-pushed the support_wasmfs_select_poll branch from 1bbcfe0 to 9eb2554 Compare November 8, 2026 14:11

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant