Skip to content

Allow changing socket blocking state#4955

Merged
RalfJung merged 1 commit intorust-lang:masterfrom
WhySoBad:network-socket-set-non-blocking
Apr 14, 2026
Merged

Allow changing socket blocking state#4955
RalfJung merged 1 commit intorust-lang:masterfrom
WhySoBad:network-socket-set-non-blocking

Conversation

@WhySoBad
Copy link
Copy Markdown
Contributor

Hi,

This pull request adds the required functionality to change the network socket blocking state through ioctl (used by the standard library) and fcntl.

Since the ioctl shim now allows changing the socket blocking state through the standard library, there are now also some tests for testing non-blocking sockets using the standard library. However, we cannot test non-blocking connects this way, as we would need mio for that.

Is it worth adding mio as a test dependency for this in a future PR? Because we already test non-blocking connects using libc.

Also, because setting the blocking state of a socket through fcntl is supported on all targets, we can now also run the libc non-blocking socket tests on MacOS targets.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two.
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Apr 12, 2026
Copy link
Copy Markdown
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! :) Just a bunch of fairly minor comments.

@rustbot author

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Apr 13, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 13, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@WhySoBad
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Apr 13, 2026
@RalfJung
Copy link
Copy Markdown
Member

This looks great, thanks! After resolving the last comment nit, please squash the commits. You can squash manually if there are multiple independent commits you want to preserve, or use ./miri squash (make sure to pick a suitable commit message). Then write @rustbot ready after you force-pushed the squashed PR.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Apr 14, 2026
@WhySoBad WhySoBad force-pushed the network-socket-set-non-blocking branch from 9218194 to 34c64c5 Compare April 14, 2026 13:57
@WhySoBad
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Apr 14, 2026
@RalfJung RalfJung enabled auto-merge April 14, 2026 14:06
@RalfJung RalfJung added this pull request to the merge queue Apr 14, 2026
@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented Apr 14, 2026

@RalfJung some test failed on the CI on the PR on my branch, can you still remove it from the merge queue?

@WhySoBad
Copy link
Copy Markdown
Contributor Author

WhySoBad commented Apr 14, 2026

I think this is a timing issue. Probably, the socket is still connecting when we're calling recv. If that's the case, we also need to loop there, but this time when ENOTCONN is returned:
https://github.com/WhySoBad/miri/actions/runs/24403117527/job/71278916255

@RalfJung RalfJung removed this pull request from the merge queue due to a manual request Apr 14, 2026
@WhySoBad
Copy link
Copy Markdown
Contributor Author

If that's the case, we also need to loop there, but this time when ENOTCONN is returned

I've now added a new test, which only tests non-blocking connect and awaits the connection using such a loop. Everywhere else I changed the tests to connect whilst the client socket is blocking and only afterwards set the socket to be non-blocking.

@RalfJung
Copy link
Copy Markdown
Member

👍 please re-squash. (You can do this preemptively for such tiny fixes if you want.)

Add shims for `fcntl` and `ioctl` to change socket blocking mode.
@WhySoBad WhySoBad force-pushed the network-socket-set-non-blocking branch from 96959f2 to 8efb5f2 Compare April 14, 2026 20:34
@RalfJung RalfJung enabled auto-merge April 14, 2026 20:50
@RalfJung RalfJung added this pull request to the merge queue Apr 14, 2026
Merged via the queue into rust-lang:master with commit 82be54a Apr 14, 2026
13 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Waiting for a review to complete label Apr 14, 2026
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.

3 participants