Skip to content

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

@softstream-link

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)

Activity

  1. added
    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and tools
    on Sep 1, 2023
  2. added
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Sep 1, 2023
  3. ChrisDenton commented on Sep 1, 2023

    @ChrisDenton
    Member

    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?

  4. softstream-link commented on Sep 1, 2023

    @softstream-link
    Author
  5. added
    T-libsRelevant to the library team, which will review and decide on the PR/issue.
    A-ioArea: `std::io`, `std::fs`, `std::net` and `std::path`
    and removed
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Sep 1, 2023
  6. ChrisDenton commented on Sep 1, 2023

    @ChrisDenton
    Member

    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_all and non-blocking TcpStream. It would help to have a very minimal example of that specific issue that can be used to inform users of the problem.

  7. scottmcm commented on Sep 1, 2023

    @scottmcm
    Member

    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 default
    

    So I'm going to close this as not a std issue.

    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.)

  8. softstream-link commented on Sep 1, 2023

    @softstream-link
    Author

    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()
  9. softstream-link commented on Sep 1, 2023

    @softstream-link
    Author

    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 default
    

    So I'm going to close this as not a std issue.

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-ioArea: `std::io`, `std::fs`, `std::net` and `std::path`T-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions