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"); +}