reduce code duplication and join in reader threads cleanly

tests/by-util/test_env.rs

@@ -293,14 +293,16 @@ fn test_simulation_of_terminal_pty_sends_eot_automatically() {
 fn test_simulation_of_terminal_pty_pipes_into_data_and_sends_eot_automatically() {
     let scene = TestScenario::new(util_name!());
 
+    let message = "Hello stdin forwarding!";
+
     let mut cmd = scene.ucmd();
     cmd.args(&["cat", "-"]);
     cmd.terminal_simulation(true);
-    cmd.pipe_in("Hello stdin forwarding!");
+    cmd.pipe_in(message);
     let child = cmd.run_no_wait();
     let out = child.wait().unwrap();
 
-    assert_eq!(String::from_utf8_lossy(out.stdout()), "Hello stdin forwarding!\r\n");
+    assert_eq!(String::from_utf8_lossy(out.stdout()), format!("{}\r\n", message));
     assert_eq!(String::from_utf8_lossy(out.stderr()), "");
 }
 

tests/common/util.rs

@@ -3,7 +3,7 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-//spell-checker: ignore (linux) rlimit prlimit coreutil ggroups uchild uncaptured scmd SHLVL canonicalized
+//spell-checker: ignore (linux) rlimit prlimit coreutil ggroups uchild uncaptured scmd SHLVL canonicalized openpty
 
 #![allow(dead_code)]
 
@@ -1429,6 +1429,26 @@ impl UCommand {
         }
     }
 
+    fn spawn_reader_thread(&self, captured_output: Option<CapturedOutput>, pty_fd_master: OwnedFd, name: String) -> Option<CapturedOutput> {
+        if let Some(mut captured_output_i) = captured_output {
+            let fd = captured_output_i.try_clone().unwrap();
+
+            let handle = std::thread::Builder::new()
+                .name(name)
+                .spawn(move ||{
+                    let mut buffer_out = String::default();
+                    Self::read_string_from_pty(pty_fd_master, &mut buffer_out);
+                    let mut out = std::fs::File::from(fd);
+                    out.write_all(buffer_out.as_bytes()).unwrap();
+                }).unwrap();
+
+            captured_output_i.reader_thread_handle = Some(handle);
+            Some(captured_output_i)
+        } else {
+            None
+        }
+    }
+
     /// Build the `std::process::Command` and apply the defaults on fields which were not specified
     /// by the user.
     ///
@@ -1585,36 +1605,8 @@ impl UCommand {
 
             stdin_pty = Some(pi_master);
 
-            if let Some(mut captured_stdout_i) = captured_stdout {
-                let fd = captured_stdout_i.try_clone().unwrap();
-
-                std::thread::Builder::new()
-                    .name("fwd_stdout".to_string())
-                    .spawn(move ||{
-                        let mut buffer_out = String::default();
-                        Self::read_string_from_pty(po_master, &mut buffer_out);
-                        let mut out = std::fs::File::from(fd);
-                        out.write_all(buffer_out.as_bytes()).unwrap();
-                        //std::mem::forget(_pi_master); // _pi_master needs to be kept alive otherwise stdin.is_terminal() fails
-                    }).unwrap();
-
-                captured_stdout = Some(captured_stdout_i);
-            }
-
-            if let Some(mut captured_stderr_i) = captured_stderr {
-                let fd = captured_stderr_i.try_clone().unwrap();
-
-                std::thread::Builder::new()
-                    .name("fwd_stderr".to_string())
-                    .spawn(move ||{
-                        let mut buffer_out = String::default();
-                        Self::read_string_from_pty(pe_master, &mut buffer_out);
-                        let mut out = std::fs::File::from(fd);
-                        out.write_all(buffer_out.as_bytes()).unwrap();
-                    }).unwrap();
-
-                captured_stderr = Some(captured_stderr_i);
-            }
+            captured_stdout = self.spawn_reader_thread(captured_stdout, po_master, "stdout_reader".to_string());
+            captured_stderr = self.spawn_reader_thread(captured_stderr, pe_master, "stderr_reader".to_string());
 
             command
                 .stdin(pi_slave)
@@ -1715,6 +1707,7 @@ impl std::fmt::Display for UCommand {
 struct CapturedOutput {
     current_file: File,
     output: tempfile::NamedTempFile, // drop last
+    reader_thread_handle: Option<thread::JoinHandle<()>>,
 }
 
 impl CapturedOutput {
@@ -1723,6 +1716,7 @@ impl CapturedOutput {
         Self {
             current_file: output.reopen().unwrap(),
             output,
+            reader_thread_handle: None,
         }
     }
 
@@ -1799,6 +1793,7 @@ impl Default for CapturedOutput {
         Self {
             current_file: file.reopen().unwrap(),
             output: file,
+            reader_thread_handle: None,
         }
     }
 }
@@ -2139,9 +2134,11 @@ impl UChild {
         };
 
         if let Some(stdout) = self.captured_stdout.as_mut() {
+            stdout.reader_thread_handle.take().map(|handle| handle.join().unwrap());
             output.stdout = stdout.output_bytes();
         }
         if let Some(stderr) = self.captured_stderr.as_mut() {
+            stderr.reader_thread_handle.take().map(|handle| handle.join().unwrap());
             output.stderr = stderr.output_bytes();
         }