subprocess tee: check process end *after* reading

python/res/util/subprocess.py

@@ -22,17 +22,26 @@ def await_process_tee(process, *out_files):
     """Wait for process to finish, "tee"-ing the subprocess' stdout into all the
     given file objects.
 
+    NB: We aren't checking if `os.write` succeeds. It succeeds if its return
+    value matches `len(bytes_)`. In other cases we might want to do something
+    smart, such as retry or raise an error. At the time of writing it is
+    uncertain what we should do, and it is assumed that data loss is acceptable.
+
     """
     out_fds = [f.fileno() for f in out_files]
     process_fd = process.stdout.fileno()
 
-    while process.poll() is None:
+    while True:
         while True:
             bytes_ = os.read(process_fd, 4096)
             if bytes_ == b"":  # check EOF
                 break
             for fd in out_fds:
                 os.write(fd, bytes_)
+
+        # Check if process terminated
+        if process.poll() is not None:
+            break
     process.stdout.close()
 
     return process.returncode

python/tests/res/util/test_subprocess.py

@@ -40,3 +40,21 @@ def test_await_process_tee(self):
         self.assertTrue(process.stdout.closed)
         self.assertEqual(cat_content, a_content)
         self.assertEqual(cat_content, b_content)
+
+    @tmpdir()
+    def test_await_process_finished_tee(self):
+        with open("a", "wb") as a_fh, open("b", "wb") as b_fh:
+            process = Popen(["/bin/cat", "/bin/cat"], stdout=PIPE)
+            process.wait()
+            await_process_tee(process, a_fh, b_fh)
+
+        with open("a", "rb") as f:
+            a_content = f.read()
+        with open("b", "rb") as f:
+            b_content = f.read()
+        with open("/bin/cat", "rb") as f:
+            cat_content = f.read()
+
+        self.assertTrue(process.stdout.closed)
+        self.assertEqual(cat_content, a_content)
+        self.assertEqual(cat_content, b_content)