Repository navigation
uutests: open simulated terminals close-on-exec - #15217
Merged
Merged
Conversation
This was referenced Oct 10, 2026
sylvestre
reviewed
Oct 10, 2026
| #[cfg(target_os = "linux")] | ||
| #[test] | ||
| fn test_simulation_of_terminal_not_inherited_by_other_commands() { | ||
| let scene = TestScenario::new("util"); |
Contributor
There was a problem hiding this comment.
sleep 1 would be enough, no?
sylvestre
reviewed
Oct 10, 2026
| #[cfg(not(any( | ||
| target_os = "linux", | ||
| target_os = "android", | ||
| target_os = "freebsd", |
Contributor
There was a problem hiding this comment.
same cfg list twice, could it be a single cfg_if! or an else branch?
Contributor
Author
There was a problem hiding this comment.
There's no cfg_if in the tree, and cfg_select! isn't available at our 1.88 MSRV. A cfg!() if/else would need both branches to compile everywhere, but OpenptFlags::CLOEXEC only exists on Linux, Android, FreeBSD and NetBSD. I can name the list once with a cfg_aliases build.rs in uutests, if you prefer.
openpty doesn't set close-on-exec, so every command that another test started while a terminal was simulated inherited its PTYs, up to six descriptors. Tests that run under a low descriptor limit then failed. Open the PTY close-on-exec through rustix where the OS supports it (Linux, Android, FreeBSD, NetBSD), and set it right after openpty elsewhere, which leaves a short window. The stdin PTY stayed open only through the copy the child inherited. Read its echo in a thread, as a terminal would, which keeps it open until the child closes it; macOS also waits at exit for an unread echo.
abendrothj
force-pushed
the
uutests-pty-cloexec
branch
from
October 11, 2026 02:51
37caa16 to
95217b2
Compare
|
GNU testsuite comparison: |
Contributor
|
Thanks for your PR |
xtqqczze
reviewed
Oct 13, 2026
| /// Open a PTY whose two ends are close-on-exec, so that commands started by | ||
| /// other tests in the meantime don't inherit them. | ||
| #[cfg(all(unix, not(target_os = "redox")))] | ||
| fn open_pty(size: Option<&libc::winsize>) -> OpenptyResult { |
Collaborator
There was a problem hiding this comment.
This fails to compile on NetBSD targets:
$ RUSTC_BOOTSTRAP=1 cargo clippy -q -Zbuild-std -p uutests --target i686-unknown-netbsd
error[E0432]: unresolved import `rustix::pty::ptsname`
--> tests/uutests/src/lib/util.rs:3057:57
|
3057 | use rustix::pty::{OpenptFlags, grantpt, openpt, ptsname, unlockpt};
| ^^^^^^^ no `ptsname` in `pty`
|
note: found an item that was configured out
--> ~/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rustix-1.1.5/src/pty.rs:135:8
|
124 | any(
| ________-
125 | | apple,
126 | | linux_like,
127 | | target_os = "freebsd",
128 | | target_os = "fuchsia",
129 | | target_os = "illumos"
130 | | )
| |_____- the item is gated here
...
135 | pub fn ptsname<Fd: AsFd, B: Into<Vec<u8>>>(fd: Fd, reuse: B) -> io::Result<CString> {
| ^^^^^^^
For more information about this error, try `rustc --explain E0432`.
Contributor
Author
There was a problem hiding this comment.
Thanks, I missed that rustix has no ptsname on NetBSD.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PTYs behind terminal simulation weren't close-on-exec, so commands other tests started in the meantime inherited them. That made the descriptor-limit tests flaky, such as sort's on #15135.
On Linux, Android, FreeBSD and NetBSD the PTY is now opened close-on-exec through rustix. Elsewhere close-on-exec is set right after
openpty, which leaves a short window. If the platform split isn't wanted, theopenpty+fcntlpath alone would do, with that window everywhere.Removing the leak breaks the harness's stdin terminal: it only stayed open because the child inherited a copy of its controller end, so "closing" stdin now hangs it up (on Linux, 6 of the 10 terminal-simulation tests in
test_env.rsfail). A thread now reads that PTY's echo, as a terminal would, which keeps it open until the child closes it. That also avoids a ~0.6s wait per test on macOS, where exit waits for the unread echo.test_simulation_of_terminal_not_inherited_by_other_commandsfails on main and passes here.test_more now kills
moreafter each pty test: some options don't quit onq(see #8429), and once the terminal stops leaking, a leftovermorespins and slows the tail tests.Closes #15216