From 9781fc5e08ff07d56b4bfcfd5f2bbf9a89ea9ae7 Mon Sep 17 00:00:00 2001 From: harshasiddartha <147021873+harshasiddartha@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:20:19 +0530 Subject: [PATCH] comm: share standard input between both operands `comm - -` locked standard input once per operand and deadlocked on the second lock, before reading anything. Hand both operands the same lock instead, so they take lines from the one stream in turn, which is what GNU comm prints for the same input. --- src/uu/comm/src/comm.rs | 33 +++++++++++++++++++++++++-------- tests/by-util/test_comm.rs | 15 +++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/uu/comm/src/comm.rs b/src/uu/comm/src/comm.rs index bf988ebc06b..fef1f8be130 100644 --- a/src/uu/comm/src/comm.rs +++ b/src/uu/comm/src/comm.rs @@ -5,11 +5,13 @@ // spell-checker:ignore (ToDO) delim mkdelim pairable +use std::cell::RefCell; use std::cmp::Ordering; use std::ffi::OsString; use std::fs::File; use std::io::{self, BufRead, BufReader, BufWriter, StdinLock, Write, stderr, stdin}; use std::path::Path; +use std::rc::Rc; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError}; use uucore::format_usage; @@ -72,14 +74,24 @@ fn line_cmp(a: &[u8], b: &[u8], use_locale: bool) -> Ordering { } } +/// The standard input, shared by every operand that names it. +type SharedStdin = Rc>>; + enum Input { - Stdin(StdinLock<'static>), + Stdin(SharedStdin), FileIn(BufReader), } impl Input { - fn stdin() -> Self { - Self::Stdin(stdin().lock()) + /// Both operands may name standard input, and then they read one stream + /// between them, taking lines from it in turn. Locking it a second time + /// would deadlock instead. + fn stdin(shared: &mut Option) -> Self { + Self::Stdin( + shared + .get_or_insert_with(|| Rc::new(RefCell::new(stdin().lock()))) + .clone(), + ) } fn from_file(f: File) -> Self { @@ -101,7 +113,7 @@ impl LineReader { let line_ending = self.line_ending.into(); let result = match &mut self.input { - Input::Stdin(r) => r.read_until(line_ending, buf), + Input::Stdin(r) => r.borrow_mut().read_until(line_ending, buf), Input::FileIn(r) => r.read_until(line_ending, buf), }; @@ -286,9 +298,13 @@ fn comm( } } -fn open_file(name: &OsString, line_ending: LineEnding) -> io::Result { +fn open_file( + name: &OsString, + line_ending: LineEnding, + shared_stdin: &mut Option, +) -> io::Result { if name == "-" { - Ok(LineReader::new(Input::stdin(), line_ending)) + Ok(LineReader::new(Input::stdin(shared_stdin), line_ending)) } else { // some platforms shows different read error // try to override the error message, but failure of it is not serious @@ -312,9 +328,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let line_ending = LineEnding::from_zero_flag(matches.get_flag(options::ZERO_TERMINATED)); let filename1 = matches.get_one::(options::FILE_1).unwrap(); let filename2 = matches.get_one::(options::FILE_2).unwrap(); - let mut f1 = open_file(filename1, line_ending) + let mut shared_stdin = None; + let mut f1 = open_file(filename1, line_ending, &mut shared_stdin) .map_err_context(|| filename1.maybe_quote().to_string())?; - let mut f2 = open_file(filename2, line_ending) + let mut f2 = open_file(filename2, line_ending, &mut shared_stdin) .map_err_context(|| filename2.maybe_quote().to_string())?; // Due to default_value(), there must be at least one value here, thus unwrap() must not panic. diff --git a/tests/by-util/test_comm.rs b/tests/by-util/test_comm.rs index 29b8e7b1816..07ec6c3a53f 100644 --- a/tests/by-util/test_comm.rs +++ b/tests/by-util/test_comm.rs @@ -4,6 +4,8 @@ // file that was distributed with this source code. // spell-checker:ignore (words) defaultcheck nocheck helpb helpz nwordb nwordwordz wordtotal +use std::time::Duration; + use uutests::new_ucmd; use uutests::util::TestScenario; #[cfg(unix)] @@ -793,3 +795,16 @@ fn test_comm_write_error_dev_full() { .fails() .stderr_is("comm: write error: No space left on device\n"); } + +#[test] +fn test_both_operands_read_stdin() { + // Both operands name standard input, so they share one stream and take + // lines from it in turn. This used to deadlock on locking it twice, hence + // the timeout. + new_ucmd!() + .args(&["-", "-"]) + .pipe_in("a\nb\nc\n") + .timeout(Duration::from_secs(30)) + .succeeds() + .stdout_only("a\n\tb\nc\n"); +}