-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fix unsound File
methods
#95469
Fix unsound File
methods
#95469
Conversation
r? @yaahc (rust-highfive has picked a reviewer for you, use r? to override) |
b716b4b
to
51061bf
Compare
NtReadFile and NtWriteFile are publicly documented and so are fine from the Windows team's perspective. I don't think trying CancelIo is worth it. Whether it will work synchronously like this is highly dependent on the machine, Windows version, configured file system drivers, etc. Better to be deterministic. |
Thanks! Using it does appear dubious at best so I'll remove it. |
6170c7f
to
e6f3191
Compare
The general approach here seems reasonable to me. However, rather than manually printing a message and aborting, could this use r=me with that change, assuming it's possible. |
@bors r+ |
📌 Commit be16d650dadc758244b38e41a766d13730707a91 has been approved by |
☔ The latest upstream changes (presumably #95653) made this pull request unmergeable. Please resolve the merge conflicts. |
be16d65
to
d2ce150
Compare
Ok, I've rebased on master so this should now be ready for merge. |
@bors r=joshtriplett |
📌 Commit d2ce150 has been approved by |
…isDenton make windows compat_fn (crudely) work on Miri With rust-lang#95469, Windows `compat_fn!` now has to be supported by Miri to even make stdout work. Unfortunately, it relies on some outside-of-Rust linker hacks (`#[link_section = ".CRT$XCU"]`) that are rather hard to make work in Miri. So I came up with this crude hack to make this stuff work in Miri regardless. It should come at no cost for regular executions, so I hope this is okay. Cc rust-lang#95627 `@ChrisDenton`
…Denton make windows compat_fn (crudely) work on Miri With rust-lang#95469, Windows `compat_fn!` now has to be supported by Miri to even make stdout work. Unfortunately, it relies on some outside-of-Rust linker hacks (`#[link_section = ".CRT$XCU"]`) that are rather hard to make work in Miri. So I came up with this crude hack to make this stuff work in Miri regardless. It should come at no cost for regular executions, so I hope this is okay. Cc rust-lang#95627 `@ChrisDenton`
…implementation This fixes #4801, where, as a result of rust-lang/rust#95469, our implementation of cat used for this test no longer works, as stdio functions on windows now can abort the process if the pipe is set to nonblocking mode. Unfortunately in windows, setting one end of the pipe to be nonblocking makes the whole thing nonblocking, so when, in tokio::process we set the child pipes to nonblocking mode, it causes serious problems for any rust program at the other end. Fixing this issue is for another day, but fixing the tests is for today.
…implementation (#4803) This fixes #4801, where, as a result of rust-lang/rust#95469, our implementation of cat used for this test no longer works, as stdio functions on windows now can abort the process if the pipe is set to nonblocking mode. Unfortunately in windows, setting one end of the pipe to be nonblocking makes the whole thing nonblocking, so when, in tokio::process we set the child pipes to nonblocking mode, it causes serious problems for any rust program at the other end. Fixing this issue is for another day, but fixing the tests is for today.
Add rust-lang#95469 to the release notes rust-lang#95469 may break programs using async file handles so it should've been noted in compatibility notes (sorry).
Rollup of 6 pull requests Successful merges: - rust-lang#98701 (Add regression test for rust-lang#50439) - rust-lang#98715 (add ice test for rust-lang#97047) - rust-lang#98753 (Fix `x dist rust-dev` on a fresh checkout) - rust-lang#98805 (Add rust-lang#95469 to the release notes) - rust-lang#98812 (feat: Add a documentation problem issue template) - rust-lang#98819 (update Miri) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
make windows compat_fn (crudely) work on Miri With rust-lang/rust#95469, Windows `compat_fn!` now has to be supported by Miri to even make stdout work. Unfortunately, it relies on some outside-of-Rust linker hacks (`#[link_section = ".CRT$XCU"]`) that are rather hard to make work in Miri. So I came up with this crude hack to make this stuff work in Miri regardless. It should come at no cost for regular executions, so I hope this is okay. Cc rust-lang/rust#95627 `@ChrisDenton`
Add fallback implementation in stdio redirection for when named pipes are not available Since Windows 9X/ME does not support creating named pipes (only connecting to remote pipes created on NT), we'll have to make do with anonymous pipes, without overlapped I/O. In particular, this means that we'll have to spawn another thread in the case where both stdout and stderr are being piped and read from (`read2`). We also use the fallback implementation on NT before 4.0, as the `Drop` impl of `AsyncPipe` needs to be able to cancel I/O via `CancelIo`. Add fallbacks for `NtReadFile` and `NtWriteFile` in `synchronous_{read, write}` These might be unsound for handles that _can_ be asynchronous on 9x/ME. See rust-lang#95469 for more info
- Add fallback implementation in stdio redirection for when named pipes are not available Since Windows 9X/ME does not support creating named pipes (only connecting to remote pipes created on NT), we'll have to make do with anonymous pipes, without overlapped I/O. In particular, this means that we'll have to spawn another thread in the case where both stdout and stderr are being piped and read from (`read2`). We also use the fallback implementation on NT before 4.0, as the `Drop` impl of `AsyncPipe` needs to be able to cancel I/O via `CancelIo`. - Add fallbacks for `NtReadFile` and `NtWriteFile` in `synchronous_{read, write}` These might be unsound for handles that _can_ be asynchronous on 9x/ME. See rust-lang#95469 for more info
- Add fallback implementation in stdio redirection for when named pipes are not available Since Windows 9X/ME does not support creating named pipes (only connecting to remote pipes created on NT), we'll have to make do with anonymous pipes, without overlapped I/O. In particular, this means that we'll have to spawn another thread in the case where both stdout and stderr are being piped and read from (`read2`). We also use the fallback implementation on NT before 4.0, as the `Drop` impl of `AsyncPipe` needs to be able to cancel I/O via `CancelIo`. - Add fallbacks for `NtReadFile` and `NtWriteFile` in `synchronous_{read, write}` These might be unsound for handles that _can_ be asynchronous on 9x/ME. See rust-lang#95469 for more info
- Add fallback implementation in stdio redirection for when named pipes are not available Since Windows 9X/ME does not support creating named pipes (only connecting to remote pipes created on NT), we'll have to make do with anonymous pipes, without overlapped I/O. In particular, this means that we'll have to spawn another thread in the case where both stdout and stderr are being piped and read from (`read2`). We also use the fallback implementation on NT before 4.0, as the `Drop` impl of `AsyncPipe` needs to be able to cancel I/O via `CancelIo`. - Add fallbacks for `NtReadFile` and `NtWriteFile` in `synchronous_{read, write}` These might be unsound for handles that _can_ be asynchronous on 9x/ME. See rust-lang#95469 for more info
- Add fallback implementation in stdio redirection for when named pipes are not available Since Windows 9X/ME does not support creating named pipes (only connecting to remote pipes created on NT), we'll have to make do with anonymous pipes, without overlapped I/O. In particular, this means that we'll have to spawn another thread in the case where both stdout and stderr are being piped and read from (`read2`). We also use the fallback implementation on NT before 4.0, as the `Drop` impl of `AsyncPipe` needs to be able to cancel I/O via `CancelIo`. - Add fallbacks for `NtReadFile` and `NtWriteFile` in `synchronous_{read, write}` These might be unsound for handles that _can_ be asynchronous on 9x/ME. See rust-lang#95469 for more info
This is a fix for #81357 (unsound
File
methods on Windows). That issue has an in-depth description of the problem.If
read
,write
,read_at
,write_at
,read_buf
or other I/O method fails to complete synchronously then they will now abort the process with the message "I/O error: operation failed to complete synchronously".