Skip to content

net: Consider having from_std panic on blocking sockets #7172

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.

Sign up for GitHub

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

Jump to bottom
Open
carllerche opened this issue Feb 24, 2025 · 2 comments
Open

net: Consider having from_std panic on blocking sockets #7172

carllerche opened this issue Feb 24, 2025 · 2 comments
Labels
A-tokio Area: The main tokio crate C-proposal Category: a proposal and request for comments M-net Module: tokio/net S-breaking-change A breaking change that requires manual coordination to be released.

Comments

@carllerche
Copy link
Member

While Tokio technically accepts and can function using blocking sockets, it usually results in poor and unexpected runtime behaviors. There is most likely no good reason to pass a blocking socket to from_std. Accidentally passing blocking sockets to from_std is a common error that results in hard-to-debug runtime behaviors.

Because Tokio has historically technically accepted blocking sockets to be passed to from_std, changing the behavior to panic on blocking sockets could be considered a breaking change. We are considering making that change anyway because we are not aware of any valid usage.

As a first step, we changed Tokio to panic when passing a blocking socket to from_std and debug assertions are enabled. Use the tokio_allow_from_blocking_fd cfg flag to disable this assertion.

If you have a valid use case for passing blocking sockets to from_std, please comment in this thread.
@carllerche carllerche added A-tokio Area: The main tokio crate C-proposal Category: a proposal and request for comments M-net Module: tokio/net S-breaking-change A breaking change that requires manual coordination to be released. labels Feb 24, 2025
@carllerche carllerche pinned this issue Feb 24, 2025
@Darksonn
Copy link
Contributor

Darksonn commented Feb 25, 2025 •
edited
Loading

This is being implemented in #7166. Please also see #5595 that tracks the problem that this proposal is intended to solve.

This change was released in Tokio v1.44.0.

@Darksonn Darksonn mentioned this issue Feb 25, 2025
Merged
Noah-Kennedy added a commit that referenced this issue Mar 5, 2025
@Noah-Kennedy
net: debug_assert on creating a tokio socket from a blocking one (#7166)
042433c 
See #5595 and #7172.

This adds a debug assertion that checks that a supplied underlying std socket is set to nonblocking mode when constructing a tokio socket object from such an object.

This only works on unix.
@Darksonn Darksonn mentioned this issue Mar 7, 2025
Merged
@bkirwi bkirwi mentioned this issue Mar 18, 2025
Merged
5 tasks
@izissise izissise mentioned this issue Mar 19, 2025
Open
bkirwi added a commit to MaterializeInc/materialize that referenced this issue Mar 19, 2025
@bkirwi
[fivetran] Use nonblocking socket for tokio (#31937)
fd7f80d 
### Motivation

Testdrive started failing for me locally due to this Tokio behaviour
change: tokio-rs/tokio#7172

~I'm unsure why this doesn't fail in CI... maybe a MacOS thing?~ I
believe this is happening locally because I'm running the code in debug
mode, and these assertions are debug-only for now. Anyways setting the
option makes things work as expected.
@bjorn3
Copy link

bjorn3 commented Mar 21, 2025

When I used from_std, I assumed tokio would set the socket as nonblocking inside the from_std function rather than requiring the user to set it.

jonhoo added a commit to jonhoo/async-bincode that referenced this issue Mar 22, 2025
@jonhoo
fix for tokio-rs/tokio#7172
031c280
lyuts added a commit to lyuts/net-route that referenced this issue Apr 9, 2025
@lyuts
Make UnixStream non-blocking.
166a3f6 
The latest upstream code is failing on macos at runtime with the
following error:

```
Registering a blocking socket with the tokio runtime is unsupported. If
you wish to do anyways, please add `--cfg tokio_allow_from_blocking_fd`
to your RUSTFLAGS. See github.com/tokio-rs/tokio/issues/7172 for
details.
```

Ref: https://github.com/tokio-rs/tokio/blob/master/tokio/src/util/blocking_check.rs#L12-L17

This change is similar to johnyburd#22 which fixed the issue for the route
listener, but it doesn't solve the issue for add/delete methods because
they end up creating their own UnixStream.

Minimal reproducible sample

```rust
use net_route::{Handle, Route};

async fn main() {
    let handle = Handle::new().unwrap();
    let route = Route::new("10.11.12.13".parse().unwrap(), 32);
    handle.add(&route).await;
}
```
nyetwurk added a commit to Blockdaemon/agave-snapshot-gossip-client that referenced this issue Apr 14, 2025
@nyetwurk
fix(#18): use ip_echo to get public_ip and shred_version
bc39a9a 
Fix error re: non-blocking and using from_std on a std TcpListener by setting it non-blocking

In debug mode:

thread 'main' panicked at src/ip_echo.rs:128:32:
Registering a blocking socket with the tokio runtime is unsupported. If you wish to do anyways, please add `--cfg tokio_allow_from_blocking_fd` to your RUSTFLAGS. See github.com/tokio-rs/tokio/issues/7172 for details.
@Xuanwo Xuanwo mentioned this issue Apr 17, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-tokio Area: The main tokio crate C-proposal Category: a proposal and request for comments M-net Module: tokio/net S-breaking-change A breaking change that requires manual coordination to be released.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@carllerche @Darksonn @bjorn3