Skip to content

Conversation

haraldh
Copy link
Contributor

@haraldh haraldh commented Mar 1, 2022

Allow polls on read and write.

See also tokio-rs/mio#1549 (comment)

Why was this limitation added?

Signed-off-by: Harald Hoyer [email protected]

Allow polls on read _and_ write.

Signed-off-by: Harald Hoyer <[email protected]>
@github-actions github-actions bot added the wasi Issues pertaining to WASI label Mar 1, 2022
@github-actions
Copy link

github-actions bot commented Mar 1, 2022

Subscribe to Label Action

cc @kubkon

This issue or pull request has been labeled: "wasi"

Thus the following users have been cc'd because of the following labels:

  • kubkon: wasi

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

haraldh added a commit to haraldh/mio that referenced this pull request Mar 1, 2022
Given, that wasmtime was fixed with
bytecodealliance/wasmtime#3866

then we can poll on POLLIN and POLLOUT with multiple subscriptions.

Signed-off-by: Harald Hoyer <[email protected]>
@haraldh haraldh marked this pull request as ready for review March 1, 2022 14:50
@pchickey
Copy link
Contributor

pchickey commented Mar 1, 2022

It appears that 7a1b7cd changed this code from the Poll structure holding mutable refs to a WasiFile to having a shared ref, so this used-at-most-once check is no longer necessary.

Copy link
Contributor

@pchickey pchickey left a comment

Choose a reason for hiding this comment

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

This code was made unnecessary by the RFC11 refactor but we didn't catch it during code review.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

I agree. With the way __wasi_poll_oneoff currently works, it should be able to handle the same file descriptor appearing multiple times.

@pchickey pchickey merged commit e8ae3c0 into bytecodealliance:main Mar 1, 2022
@pchickey
Copy link
Contributor

pchickey commented Mar 1, 2022

Thanks @haraldh !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants