Skip to content

Commit 7963769

Browse files
committed
stdbuf: use exec instead of forking
Forking creates a new PID and it not compatible with GNU coreutils implementation. - use exec instead of forking - add test verifying that execvp is used Fixes #9066 Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>
1 parent 2000af8 commit 7963769

File tree

2 files changed

+88
-51
lines changed

2 files changed

+88
-51
lines changed

src/uu/stdbuf/src/stdbuf.rs

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,13 @@
77

88
use clap::{Arg, ArgAction, ArgMatches, Command};
99
use std::ffi::OsString;
10+
use std::os::unix::process::CommandExt;
1011
use std::path::PathBuf;
1112
use std::process;
1213
use tempfile::TempDir;
1314
use tempfile::tempdir;
1415
use thiserror::Error;
15-
use uucore::error::{FromIo, UResult, USimpleError, UUsageError};
16+
use uucore::error::{UResult, USimpleError, UUsageError};
1617
use uucore::format_usage;
1718
use uucore::parser::parse_size::parse_size_u64;
1819
use uucore::translate;
@@ -205,55 +206,22 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
205206
set_command_env(&mut command, "_STDBUF_E", &options.stderr);
206207
command.args(command_params);
207208

208-
let mut process = match command.spawn() {
209-
Ok(p) => p,
210-
Err(e) => {
211-
return match e.kind() {
212-
std::io::ErrorKind::PermissionDenied => Err(USimpleError::new(
213-
126,
214-
translate!("stdbuf-error-permission-denied"),
215-
)),
216-
std::io::ErrorKind::NotFound => Err(USimpleError::new(
217-
127,
218-
translate!("stdbuf-error-no-such-file"),
219-
)),
220-
_ => Err(USimpleError::new(
221-
1,
222-
translate!("stdbuf-error-failed-to-execute", "error" => e),
223-
)),
224-
};
225-
}
226-
};
227-
228-
let status = process.wait().map_err_context(String::new)?;
229-
match status.code() {
230-
Some(i) => {
231-
if i == 0 {
232-
Ok(())
233-
} else {
234-
Err(i.into())
235-
}
236-
}
237-
None => {
238-
#[cfg(unix)]
239-
{
240-
use std::os::unix::process::ExitStatusExt;
241-
let signal_msg = status
242-
.signal()
243-
.map_or_else(|| "unknown".to_string(), |s| s.to_string());
244-
Err(USimpleError::new(
245-
1,
246-
translate!("stdbuf-error-killed-by-signal", "signal" => signal_msg),
247-
))
248-
}
249-
#[cfg(not(unix))]
250-
{
251-
Err(USimpleError::new(
252-
1,
253-
"process terminated abnormally".to_string(),
254-
))
255-
}
256-
}
209+
// Replace the current process with the target program (no fork) using exec.
210+
let e = command.exec();
211+
// exec() only returns if there was an error
212+
match e.kind() {
213+
std::io::ErrorKind::PermissionDenied => Err(USimpleError::new(
214+
126,
215+
translate!("stdbuf-error-permission-denied"),
216+
)),
217+
std::io::ErrorKind::NotFound => Err(USimpleError::new(
218+
127,
219+
translate!("stdbuf-error-no-such-file"),
220+
)),
221+
_ => Err(USimpleError::new(
222+
1,
223+
translate!("stdbuf-error-failed-to-execute", "error" => e),
224+
)),
257225
}
258226
}
259227

tests/by-util/test_stdbuf.rs

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
5-
// spell-checker:ignore dyld dylib setvbuf
5+
// spell-checker:ignore cmdline dyld dylib PDEATHSIG setvbuf
66
#[cfg(target_os = "linux")]
77
use uutests::at_and_ucmd;
88
use uutests::new_ucmd;
@@ -247,3 +247,72 @@ fn test_stdbuf_non_utf8_paths() {
247247
.succeeds()
248248
.stdout_is("test content for stdbuf\n");
249249
}
250+
251+
#[test]
252+
#[cfg(target_os = "linux")]
253+
fn test_stdbuf_no_fork_regression() {
254+
// Regression test for issue #9066: https://github.com/uutils/coreutils/issues/9066
255+
// The original stdbuf implementation used fork+spawn which broke signal handling
256+
// and PR_SET_PDEATHSIG. This test verifies that stdbuf uses exec() instead.
257+
// With fork: stdbuf process would remain visible in process list
258+
// With exec: stdbuf process is replaced by target command (GNU compatible)
259+
260+
use std::process::{Command, Stdio};
261+
use std::thread;
262+
use std::time::Duration;
263+
264+
let scene = TestScenario::new(util_name!());
265+
266+
// Start stdbuf with a long-running command
267+
let mut child = Command::new(&scene.bin_path)
268+
.args(["stdbuf", "-o0", "sleep", "3"])
269+
.stdout(Stdio::null())
270+
.stderr(Stdio::null())
271+
.spawn()
272+
.expect("Failed to start stdbuf");
273+
274+
let child_pid = child.id();
275+
276+
// Poll until exec happens or timeout
277+
let cmdline_path = format!("/proc/{child_pid}/cmdline");
278+
let timeout = Duration::from_secs(2);
279+
let poll_interval = Duration::from_millis(10);
280+
let start_time = std::time::Instant::now();
281+
282+
let command_name = loop {
283+
if start_time.elapsed() > timeout {
284+
child.kill().ok();
285+
panic!("TIMEOUT: Process {child_pid} did not respond within {timeout:?}");
286+
}
287+
288+
if let Ok(cmdline) = std::fs::read_to_string(&cmdline_path) {
289+
let cmd_parts: Vec<&str> = cmdline.split('\0').collect();
290+
let name = cmd_parts.first().map_or("", |v| v);
291+
292+
// Wait for exec to complete (process name changes from original binary to target)
293+
// Handle both multicall binary (coreutils) and individual utilities (stdbuf)
294+
if !name.contains("coreutils") && !name.contains("stdbuf") && !name.is_empty() {
295+
break name.to_string();
296+
}
297+
}
298+
299+
thread::sleep(poll_interval);
300+
};
301+
302+
// The loop already waited for exec (no longer original binary), so this should always pass
303+
// But keep the assertion as a safety check and clear documentation
304+
assert!(
305+
!command_name.contains("coreutils") && !command_name.contains("stdbuf"),
306+
"REGRESSION: Process {child_pid} is still original binary (coreutils or stdbuf) - fork() used instead of exec()"
307+
);
308+
309+
// Ensure we're running the expected target command
310+
assert!(
311+
command_name.contains("sleep"),
312+
"Expected 'sleep' command at PID {child_pid}, got: {command_name}"
313+
);
314+
315+
// Cleanup
316+
child.kill().ok();
317+
child.wait().ok();
318+
}

0 commit comments

Comments
 (0)