Port polling operations to wasip2 #608
Conversation
catamorphism
force-pushed
the
wasip2-porting-poll
branch
2 times, most recently
from
a6b6187 to
e1f5732
Compare
There was a problem hiding this comment.
in poll_wasip2() I think that every pollable not on the ready list gets leaked. In general I think its possible to hold onto and reuse pollables for streams (and, in prior prs, streams for files/sockets) longer than this implementation does at the moment. We can leave pollable reuse for a future PR but we should fix the leaks in this one, if possible.
| __wasi_event_t ev; | ||
| __wasi_errno_t error = __wasi_poll_oneoff(&sub, &ev, 1, &nevents); | ||
| return error == 0 && ev.error == 0 ? 0 : ENOTSUP; | ||
| poll_method_pollable_block(poll_borrow_pollable(pollable)); |
There was a problem hiding this comment.
The pollable gets leaked here - need to drop it.
There was a problem hiding this comment.
Fixed in ca323cc
| } | ||
| } else { | ||
| } else if (state->entry->tag == DESCRIPTOR_TABLE_ENTRY_FILE_STREAM) { | ||
| poll_pollable_drop_own(state->pollable); |
There was a problem hiding this comment.
This file originally used only tabs, and some of your changes introduce spaces. I don't care one way or the other but its helpful if its consistent in the file
There was a problem hiding this comment.
Replaced tabs with spaces in 40b71f6
catamorphism
force-pushed
the
wasip2-porting-poll
branch
2 times, most recently
from
5e0771a to
c42c2b9
Compare
c42c2b9 should fix that. |
catamorphism
force-pushed
the
wasip2-porting-poll
branch
from
ca323cc to
ccdfff3
Compare
At this point, there should be no further uses of the preview1 component adapter for polling methods (__wasi_poll_oneoff()) when __wasilibc_use_wasip2 is defined.
catamorphism
force-pushed
the
wasip2-porting-poll
branch
from
ccdfff3 to
7b186b5
Compare
|
Rebased, ready for (further) review now! |
catamorphism
changed the title
There was a problem hiding this comment.
LGTM. The biggest thing is that we need better testing of pollables, but its difficult to impossible to do that from inside a single program, especially without sockets. But thats a big problem I don't want to gate this on, so lets get this landed and chat about how to approach that as follow-on work.
Port poll() to use wasip2 poll method in all cases
At this point, there should be no further uses of the preview1
component adapter for polling methods (__wasi_poll_oneoff())
when __wasilibc_use_wasip2 is defined.