Skip to content
This repository was archived by the owner on Sep 8, 2025. It is now read-only.

Conversation

@rvolosatovs
Copy link
Member

@rvolosatovs rvolosatovs commented Feb 21, 2025

This is a follow-up on #1 and #17 , which ensures the wasi:sockets TCP implementation is complete and fully tested using the adapted wasip2 test suite.

There's only one item I left a TODO about, which does not seem specific to TCP.
It appears that SinkExt::close has no effect on guest streams, which the wasip2 TCP suite tests for. Not sure if that's desired behavior or not, but it does necessarily look like a blocker for me.

Note, in current implementation streams (e.g. returned by listen) are lazy and do not do any work unless they're polled. That means, for example, that calls to connect and StreamExt::next on the stream returned by listen must happen concurrently. Calling the two in sync order may succeed, but that behavior cannot be depended upon. I believe this aligns with the current async design

Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs
Copy link
Member Author

rvolosatovs commented Feb 21, 2025

Hmm, looks like a test hangs on Linux, but not on Mac, going to investigate... The issue was the send buffer being full, fixed by receiving from concurrently with sending. I'm guessing this behavior worked for wasip2, because in that case the amount of bytes sent was derived from check_write return value, effectively just being much smaller

@rvolosatovs rvolosatovs marked this pull request as draft February 21, 2025 13:11
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs rvolosatovs marked this pull request as ready for review February 21, 2025 13:47
@rvolosatovs rvolosatovs marked this pull request as draft February 21, 2025 13:59
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs rvolosatovs marked this pull request as ready for review February 21, 2025 14:49
@dicej dicej merged commit e8e6ac1 into main Feb 21, 2025
25 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants