Repository navigation
lack of documentation on the issue that using std::io::Write::write_all with a std::net::TcpStream results in undefined behavior when non_blocking(true) #115451
Description
Activity
- addedA-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and tools
on Sep 1, 2023 - addedneeds-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triagingThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
on Sep 1, 2023 It would help to have more details here. When you say "undefined behavior" are you meaning a Rust memory safety issue? Or inconsistent or unexpected behaviour?
Can you show some code that gives an example?
original post on user forum with a solution - https://users.rust-lang.org/t/sending-data-over-a-non-blocking-socket-corrupts-data/99240
Steps to replicate - https://hackmd.io/@UiZWksNWRxCyfvxz5cAG5w/ryM0iOJR3
- addedT-libsRelevant to the library team, which will review and decide on the PR/issue.Relevant to the library team, which will review and decide on the PR/issue.A-ioArea: `std::io`, `std::fs`, `std::net` and `std::path`Area: `std::io`, `std::fs`, `std::net` and `std::path`and removedneeds-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triagingThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
on Sep 1, 2023 I don't think there's any UB in the standard library itself. There are a number of issues with the code, that helpful people on the user's forum have pointed out, but not with the standard library itself.
There might be a documentation issue here with using
write_alland non-blockingTcpStream. It would help to have a very minimal example of that specific issue that can be used to inform users of the problem.The steps to replicate contain the following as part of
read_frame:let mut buf: [u8; MAX_MESSAGE_SIZE] = unsafe { MaybeUninit::uninit().assume_init() };
That's 100% insta-UB, as the warning says in https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f15d743f0a7edd56eb2aca384838643e:
warning: the type `[u8; 10]` does not permit being left uninitialized --> src/main.rs:4:52 | 4 | let mut buf: [u8; MAX_MESSAGE_SIZE] = unsafe { MaybeUninit::uninit().assume_init() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | this code causes undefined behavior when executed | help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done | = note: integers must be initialized = note: `#[warn(invalid_value)]` on by defaultSo I'm going to close this as not a
stdissue.Feel free to re-open with a minimal repro that doesn't contain UB.
(It's possible this appeared to work in past rust, but that when #111999 changed how we emit code for byte arrays it allowed LLVM to take advantage of the UB.)
Sorry for some reason I don't see the re-open button for this issue. I am hoping you can reopen if you find below sufficient.
Here is the most basic block of code that will cause "UB" I don't want to split hair on what "UB" means but in my case it just means you don't get what you expect, hence you get something that Is not clearly defined.
let mut stream = TcpStream::connect(add).unwrap(); stream.nonblocking(true).unwrap() let buf = [1,2,3,4,5]; stream.write_all(&buf[..]); // THIS LINE WILL sometime propagate a WouldBlock error kind however it would have written X number of bytes to the steam with out a way too tell if X is =0 or X<buf.len()
The steps to replicate contain the following as part of
read_frame:let mut buf: [u8; MAX_MESSAGE_SIZE] = unsafe { MaybeUninit::uninit().assume_init() };
That's 100% insta-UB, as the warning says in https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f15d743f0a7edd56eb2aca384838643e:
warning: the type `[u8; 10]` does not permit being left uninitialized --> src/main.rs:4:52 | 4 | let mut buf: [u8; MAX_MESSAGE_SIZE] = unsafe { MaybeUninit::uninit().assume_init() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | this code causes undefined behavior when executed | help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done | = note: integers must be initialized = note: `#[warn(invalid_value)]` on by defaultSo I'm going to close this as not a
stdissue.Feel free to re-open with a minimal repro that doesn't contain UB.
(It's possible this appeared to work in past rust, but that when #111999 changed how we emit code for byte arrays it allowed LLVM to take advantage of the UB.)
changing this to SAFE init does not eliminate the issue of write_all not writing_all when the socket is notblocking, i am not sure exactly why it is unsafe to write to an uninitiated slice since the write from the socket will initialize it. I am just not aware of how else it can be done without the cost penalty, particularly since the read_buf(BorrowedCursor) 'suite' is still not stable.
Location
lack of documentation on the issue that using std::io::Write::write_all with a std::net::TcpStream results in undefined behavior when non_blocking(true)
Summary
lack of documentation on the issue that using std::io::Write::write_all with a std::net::TcpStream results in undefined behavior when non_blocking(true)