Skip to content
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

TcpStream::connect_timeout with Duration::MAX never succeeds on Windows #112405

Closed
hartwork opened this issue Jun 7, 2023 · 3 comments · Fixed by #112464
Closed

TcpStream::connect_timeout with Duration::MAX never succeeds on Windows #112405

hartwork opened this issue Jun 7, 2023 · 3 comments · Fixed by #112464
Assignees
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@hartwork
Copy link

hartwork commented Jun 7, 2023

Hello! 👋

I tried this code on Windows:

use std::net::{TcpStream, ToSocketAddrs};
use std::time::Duration;

fn main() {
    let address_result = "google.com:80".to_socket_addrs();
    let address = address_result.unwrap().next().unwrap();
    TcpStream::connect_timeout(&address, Duration::MAX).expect("Google is offline?!");
    println!("Never gets here on Windows (https://github.com/rust-lang/rust/issues/112405).");
}

I expected to see this happen: Success for an available address like google.com:80 and blocking forever (or until TCP timeout) for hostname.invalid:80 just like on Linux.

Instead, this happened: the call never succeeds, not even for google.com:80 on Windows.

Related: hartwork/rust-for-it@5056e69

Meta

rustc --version --verbose:

# rustc --version --verbose
rustc 1.69.0 (84c898d65 2023-04-16)
binary: rustc
commit-hash: 84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc
commit-date: 2023-04-16
host: x86_64-pc-windows-msvc
release: 1.69.0
LLVM version: 15.0.7
@hartwork hartwork added the C-bug Category: This is a bug. label Jun 7, 2023
@jyn514 jyn514 added O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue. A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` labels Jun 8, 2023
@ChrisDenton
Copy link
Member

Thanks for the report! My best guess from quickly glancing at the code is that the problem is here:

let mut timeout = c::timeval {
tv_sec: timeout.as_secs() as c_long,
tv_usec: (timeout.subsec_nanos() / 1000) as c_long,
};

This is using as to convert u64 seconds into an i32 (aka c_long). Which means that it can become a negative timeout. If I'm right then the fix here would be to limit the timeout to i32::MAX or less.

@ChrisDenton ChrisDenton added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Jun 8, 2023
@eval-exec
Copy link
Contributor

@rustbot claim

@hartwork
Copy link
Author

@ChrisDenton @eval-exec thanks for your work on this and pushing it over the finish line! 👍 🙏

@rust-lang rust-lang deleted a comment Jun 20, 2023
@rust-lang rust-lang deleted a comment Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants