Skip to content

net: fix non-blocking read/write#20438

Merged
spytheman merged 18 commits intovlang:masterfrom
kbkpbot:new-net-fix
Feb 8, 2024
Merged

net: fix non-blocking read/write#20438
spytheman merged 18 commits intovlang:masterfrom
kbkpbot:new-net-fix

Conversation

@kbkpbot
Copy link
Copy Markdown
Contributor

@kbkpbot kbkpbot commented Jan 8, 2024

Adding set_blocking(true) to net.dial_tcp(), net.dial_tcp_with_bind(), net.accept().
This will ensure that the upper net apps do not need to modify.

For non-blocking usage, after create a TCPConn via net.dial_tcp(), net.dial_tcp_with_bind(), net.accept(), should use set_blocking(false) change to non-blocking mode.

Both blocking/non-blocking apps can use the same read/write now.

@kbkpbot
Copy link
Copy Markdown
Contributor Author

kbkpbot commented Jan 10, 2024

Why I can't see the full log of FreeBSD Code CI ?
https://github.com/vlang/v/pull/20438/checks?check_run_id=20289394271

Comment thread vlib/net/tcp_test.v
@spytheman
Copy link
Copy Markdown
Contributor

Why I can't see the full log of FreeBSD Code CI ? https://github.com/vlang/v/pull/20438/checks?check_run_id=20289394271

See it here: https://cirrus-ci.com/task/4822104995004416 (it is linked at the bottom of the CI check page).

@hholst80
Copy link
Copy Markdown
Contributor

I can test this later this afternoon on FreeBSD and Alpine without ipv6 and comment. I would like to avoid the call to set_blocking(true) if possible and have it be blocking by default.

Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
@Casper64
Copy link
Copy Markdown
Contributor

If the default behaviour is changed from non-blocking to blocking sockets it should add conn.set_blocking(false) for code that depends on non-blocking sockets. For example the picoev and x.vweb modules will break after this is merged.

@kbkpbot
Copy link
Copy Markdown
Contributor Author

kbkpbot commented Jan 10, 2024

If the default behaviour is changed from non-blocking to blocking sockets it should add conn.set_blocking(false) for code that depends on non-blocking sockets. For example the picoev and x.vweb modules will break after this is merged.

TcpSocket is still non-blocking default.
Only the TcpConn, after calling .accept(), .dial_tcp_with_bind(), .dial_tcp(), will become blocking.

Because vweb call .accept_only(), it should add conn.set_blocking(true) manually .

Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
Comment thread vlib/net/tcp.c.v Outdated
@spytheman
Copy link
Copy Markdown
Contributor

@kbkpbot I've merged #20472 . If you rebase, I think it may fix part of the CI fail on FreeBSD.

@spytheman
Copy link
Copy Markdown
Contributor

(rebased over master, should solve FreeBSD failure)

@kbkpbot
Copy link
Copy Markdown
Contributor Author

kbkpbot commented Jan 13, 2024

I think this patch is done....

@shove70 shove70 force-pushed the new-net-fix branch 6 times, most recently from e6a7cba to 2a77e5a Compare January 24, 2024 03:49
@spytheman spytheman merged commit a9ebab0 into vlang:master Feb 8, 2024
@kbkpbot kbkpbot deleted the new-net-fix branch October 3, 2024 01:02
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.

6 participants