Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 20, 2025

This change improves the implementation of poll() to support blocking when called from threads (see #25523) and moves the select-based-on-poll implementation from being wasmfs specific to be always being used.

Also, add testing for blocking version of poll by duplicating the blocking select test.

@sbc100 sbc100 force-pushed the poll_blocking branch 2 times, most recently from b8d0829 to 9ec78d9 Compare December 22, 2025 03:15
@sbc100 sbc100 marked this pull request as ready for review December 22, 2025 03:20
@sbc100 sbc100 requested review from dschuff and kripken December 22, 2025 03:20
@sbc100 sbc100 force-pushed the poll_blocking branch 8 times, most recently from d44829d to fa8fc29 Compare December 22, 2025 05:37
Copy link
Contributor

@ktock ktock left a comment

Choose a reason for hiding this comment

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

Thanks!

@sbc100 sbc100 force-pushed the poll_blocking branch 3 times, most recently from 7201094 to 4f8ad00 Compare December 22, 2025 19:42
@sbc100 sbc100 requested a review from brendandahl December 22, 2025 19:43
@sbc100 sbc100 enabled auto-merge (squash) December 22, 2025 20:55
Copy link
Member

@aheejin aheejin left a comment

Choose a reason for hiding this comment

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

Looks like a nice simplification! A few nits below. I can't "approve" because it is set to auto-merge...

This change improves the implementation of poll() to support blocking
when called from threads (see emscripten-core#25523) and moves the select-based-on-poll
implemenation from being wasmfs specific to be always being used.
@sbc100 sbc100 disabled auto-merge December 23, 2025 16:49
@sbc100 sbc100 merged commit eb4a4d0 into emscripten-core:main Dec 23, 2025
15 of 17 checks passed
@sbc100 sbc100 deleted the poll_blocking branch December 23, 2025 17:02
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).

We can use this new mode to replace the bespoke proxying code that
exists for dlsync as well as the poll system call.  For the poll system
call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can now be
completely removed.

I'm still not sure how best to name this new mode, and I imagine it will
be fairly rare that it is used.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).

We can use this new mode to replace the bespoke proxying code that
exists for dlsync as well as the poll system call.  For the poll system
call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can now be
completely removed.

I'm still not sure how best to name this new mode, and I imagine it will
be fairly rare that it is used.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).

We can use this new mode to replace the bespoke proxying code that
exists for dlsync as well as the poll system call.  For the poll system
call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can now be
completely removed.

I'm still not sure how best to name this new mode, and I imagine it will
be fairly rare that it is used.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).

We can use this new mode to replace the bespoke proxying code that
exists for `dlsync` as well as the `poll` system call.  For the poll
system call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can
now be completely removed.

In addition to this, because the `poll` syscall is now marked as
`__async` it automatically works under JSPI too.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).  The new behaviour is triggered by marking a
functions as both `__proxy: 'sync'` and also `__async: true`.

We can use this new mode to replace the bespoke proxying code that
exists for `dlsync` as well as the `poll` system call.  For the poll
system call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can
now be completely removed.

In addition to this, because the `poll` syscall is now marked as
`__async` it automatically works under JSPI too.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.

Fixes: emscripten-core#25970
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).  The new behaviour is triggered by marking a
functions as both `__proxy: 'sync'` and also `__async: true`.

We can use this new mode to replace the bespoke proxying code that
exists for `dlsync` as well as the `poll` system call.  For the poll
system call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can
now be completely removed.

In addition to this, because the `poll` syscall is now marked as
`__async` it automatically works under JSPI too.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.

Fixes: emscripten-core#25970
sbc100 added a commit to sbc100/emscripten that referenced this pull request Dec 23, 2025
This new mode allows the proxied work itself to be asynchronous (i.e.
return a promise).  The new behaviour is triggered by marking a
functions as both `__proxy: 'sync'` and also `__async: true`.

We can use this new mode to replace the bespoke proxying code that
exists for `dlsync` as well as the `poll` system call.  For the poll
system call this bespoke code was added in emscripten-core#25523 and emscripten-core#25990, but can
now be completely removed.

In addition to this, because the `poll` syscall is now marked as
`__async` it automatically works under JSPI too.

One downside of this new mode is that the proxied work requires the main
thread event loop to run.  Unlike the synchronous proxied work that can
complete even when we are deeply nested.

Fixes: emscripten-core#25970
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.

3 participants