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

std::net::TcpStream::connect_timeout on vxworks #127018

Closed
biabbas opened this issue Jun 27, 2024 · 4 comments · Fixed by #127300
Closed

std::net::TcpStream::connect_timeout on vxworks #127018

biabbas opened this issue Jun 27, 2024 · 4 comments · Fixed by #127300
Labels
C-bug Category: This is a bug. O-vxworks Target: when they made us, they called us Curiosity, and Spirit, and told us to tell you hello T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@biabbas
Copy link
Contributor

biabbas commented Jun 27, 2024

I'm tracking a issue with connect_timeout function on rust for vxworks. The function is also accepting connections on a non responsive server port. I get errors on unreachable host, but with host that are reachable any connection to a non existent server port is not resulting in an error.

example program:

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

fn main() {
    let sock_addr: SocketAddr = std::env::args()
        .into_iter()
        .nth(1)
        .unwrap()
        .parse()
        .unwrap();
    dbg!(sock_addr);
    let timeout = Duration::from_secs(5);
    let maybe_stream = TcpStream::connect_timeout(&sock_addr, timeout);
    match maybe_stream {
        Ok(_stream) => println!("Successfully connected to server"),
        Err(err) => eprintln!("Failed to connect to server: {:?}", err),
    };
}

Results:
#./exe 127.0.0.1:90 /// No server at this port
Successfully connected to server
#./exe 192.0.0.1:20 //// Host is unreachable so correct behaviour.
Failed to connect to server
#./exe 172.16.2.223:8002 //Host is reachable, but there's no server at this port
Successfully connected to server

This is not observed with the connect function.
I don't see a specified folder in library/std/src/sys/pal for vxworks. I found only one file for vxworks library/std/src/sys/pal/unix/process/process_vxworks.rs files. Does vxworks fall back on unix implementations for these functionalities? Any leads on how to proceed would be really helpful.

@biabbas biabbas added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 27, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Jun 27, 2024

@biabbas I don't mean to quibble much but tracking issues are not for "tracking" bugs, they are for tracking long-term projects that need issues-that-connect-issues.

@workingjubilee workingjubilee added O-vxworks Target: when they made us, they called us Curiosity, and Spirit, and told us to tell you hello C-bug Category: This is a bug. and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Jun 27, 2024
@workingjubilee
Copy link
Member

I don't see a specified folder in library/std/src/sys/pal for vxworks. I found only one file for vxworks library/std/src/sys/pal/unix/process/process_vxworks.rs files. Does vxworks fall back on unix implementations for these functionalities? Any leads on how to proceed would be really helpful.

Yes, see #77666 for why: basically our support was previously a trivial copy of the Unix support. The result may be buggy, because we do not really run CI against VxWorks. We are happy to accept patches, and even happier to have maintainers who are interested in repairing the target on a regular basis.

@Noratrieb Noratrieb changed the title Tracking Issue for std::net::TcpStream::connect_timeout for vxworks std::net::TcpStream::connect_timeout on vxworks Jun 27, 2024
@biabbas
Copy link
Contributor Author

biabbas commented Jun 28, 2024

I don't see a specified folder in library/std/src/sys/pal for vxworks. I found only one file for vxworks library/std/src/sys/pal/unix/process/process_vxworks.rs files. Does vxworks fall back on unix implementations for these functionalities? Any leads on how to proceed would be really helpful.

Yes, see #77666 for why: basically our support was previously a trivial copy of the Unix support. The result may be buggy, because we do not really run CI against VxWorks. We are happy to accept patches, and even happier to have maintainers who are interested in repairing the target on a regular basis.

Thank You. I'll be continuously working on vxworks rust. So I'll be able to check this on a regular basis. Since rust 1.6 we've been maintaining a custom patch for rust build for vxworks. I'll update it for nightly and create a PR. I would like to be a code maintainer for this.

@workingjubilee workingjubilee added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jun 28, 2024
@workingjubilee
Copy link
Member

@biabbas All you have to do to be listed as a maintainer is to add a doc using this template in rustc/src/platform-support/

https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/src/doc/rustc/src/platform-support/TEMPLATE.md

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#121364 (Implement lint against ambiguous negative literals)
 - rust-lang#127300 (Fix connect timeout for non-linux targets, read readiness of socket connection, Read readiness to detect errors. `Fixes rust-lang#127018`)
 - rust-lang#128138 (`#[naked]`: use an allowlist for allowed options on `asm!` in naked functions)
 - rust-lang#128158 (std: unsafe-wrap personality::gcc)
 - rust-lang#128171 (Make sure that args are compatible in `resolve_associated_item`)
 - rust-lang#128172 (Don't ICE if HIR and middle types disagree in borrowck error reporting)
 - rust-lang#128173 (Remove crashes for misuses of intrinsics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in d1070df Jul 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2024
Rollup merge of rust-lang#127300 - biabbas:fix_connect_timeout, r=tgross35

Fix connect timeout for non-linux targets, read readiness of socket connection, Read readiness to detect errors. `Fixes rust-lang#127018`

Fixes rust-lang#127018
Connect_timeout would call `poll` and check `pollfd.revents` for POLLHUP error, rather that checking readiness. This behavior was meant for Linux as it returns POLLHUP | POLLOUT | POLLERR in case of errors. But on targets that do not return POLLHUP in `pollfd.revents`, this would indicate a false success and result in this issue. To resolve this we will check readiness of socket using  `getsockopt():`  and return success from connect_timeout when there are no errors.
Changes were tested on Linux and an rtos.
![Screenshot 2024-07-04 105820](https://github.com/rust-lang/rust/assets/88673422/5ef5a87f-f2af-4fb7-98da-7612d5e27e9a)
Thank you.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-vxworks Target: when they made us, they called us Curiosity, and Spirit, and told us to tell you hello T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
2 participants