alexcrichton commented on issue #7243:
Thanks for this! I've got a few comments on some API design decisions as well as their implications on the implementation:
Primarily I think it would be best to avoid the usage of
Mutex<T>in the implementation. I feel like it's best to reflect the concurrency of the underlying OS which is that mutexes aren't required and concurrent calls to various bits and pieces are ok. I realize that mutexes aren't held during blocking I/O operations as a part of this PR, but that effectively means that the lock is overhead and I'd prefer to do away with it. I tested this out locally and the main issue that I ran into was that the inbound/outbound streams need access to theremote_address()of the state of the socket. This is problematic because, theoretically, astart_connectcan race with asendorreceivecall, meaning that aMutexis basically required to arbitrate that race (or something atomic like that). Could this perhaps be altered at the API level where an inbound/outbound stream is returned fromfinish-connectin addition tofinish-bind? That way the streams frombindare never connected and the streams fromconnectare always connected. That may have weird implications if all the streams are used, though, so I'm not 100% sure.Bikeshed-wise the name
inbound-*andoutbound-*are inconsistent withwasi:io/streamsusage ofinput-*andoutput-*. Is it perhaps worth it to use the same terminology?Bikeshed-wise the
oubound-datagram-streamtype is similar tooutput-streambut has a different mechanism for receiving data. Theoutput-streamtype for example has acheck-writemethod which returns how much can be written and only then canwritebe called. Additionallywritetraps if too much is given to it. The main rationale for this design decision is that it avoids the need for callees to receive a possibly unbounded amount of data forcibly within their module (since callingsendwill copy over all the data into the second module if there is one). This may best be solved with acheck-writelookalike foroutbound-datagram-streamas well perhaps?
badeend commented on issue #7243:
I think it would be best to avoid the usage of Mutex<T> in the implementation.
I agree, yet I couldn't figure out how to make it work.
My initial version hadbindandconnectboth return streams. (Also I allowed connect to be called multiple times, but I didn't want to pollute this PR too much).
In that setup, callingconnecthas to invalidate the previously returned streams somehow. At which point I ended up at Mutex again. Allowing the previous streams to remain somehow usable might be easier to implement, but is arguably a worse API design.
Bikeshed-wise the name inbound-* and outbound-* are inconsistent with wasi:io/streams usage of input-* and output-*. Is it perhaps worth it to use the same terminology?
I (tried to) use the wasi-http naming scheme, as they feel more natural to the domain at hand (networking) as opposed to input&output.
Sidenote, I just noticed that wasi-http actually uses slightly different terminology (incoming&outgoing) :no_mouth:For the stream types, I'm fine with either naming scheme. They're temporary anyway.
For the datagram types themselves I do prefer to keep calling them incoming&outgoing-datagram rather than input&output-datagram.
The output-stream type for example has a check-write method which returns how much can be written and only then can write be called
Good point. I'll look into it.
The main rationale for this design decision is that it avoids the need for callees to receive a possibly unbounded amount of data forcibly within their module
Even with a
check-writemethod onoutbound-datagram-streamthat can still happen, right. I assumecheck-writewill return the amount of datagrams that can be sent. But each datagram can still contain an unbounded number of bytes in thedatafield.
alexcrichton commented on issue #7243:
One way to get invalidation working without
Mutexis to have a generational counter stored in each incoming/outgoing stream which is compared to the "current generation", an atomic counter, stored within theArc. That way invalidation would be "increment the generation" and that may work out?For naming ok sounds like we're already a bit inconsistent, so I think it's ok to keep the names here :+1:
For
check-writeyou're correct that you can always still pass in more data, but implementations can trap when this happens vs being expected to work correctly. I also think that for datagrams the number returned from check-write should probably be "size of all datagrams summed up" since you're right that the size of an individual datagram can change. Unless we can place a hard cap on the size of the datagram though in which case implementations could trap if any individual datagram is too large.
badeend commented on issue #7243:
Potentially answering my own question:
calling connect has to invalidate the previously returned streams somehow
Rather than performing validation in the *-stream code, maybe we can require the consumer to have dropped all prior child streams before calling
connect(again) and trap otherwise. That way it should be impossible to have multiple competing streams active at the same time.
alexcrichton commented on issue #7243:
Ah I think we raced there a bit, but I like your idea more of requiring the streams are dropped than my idea of a generation counter
badeend commented on issue #7243:
This is ready for review again.
I have:
- Updated the API to disallow multiple pairs of active streams.
- Updated the API to mostly match wasi-io's
input-stream&output-stream.- Removed the mutex
- Renamed the resources to align with wasi-http.
badeend commented on issue #7243:
Alright, looks like the MacOS issue is fixed now
Last updated: Dec 13 2025 at 19:03 UTC