Skip to content

wasip3 TCP implementation #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 21, 2025
Merged

wasip3 TCP implementation #20

merged 4 commits into from
Feb 21, 2025

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

@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 <[email protected]>
@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
@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 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.

2 participants