FIXUP: join the thread before writing output

dangerzone/isolation_provider/base.py

@@ -77,9 +77,9 @@ def read_int(f: IO[bytes]) -> int:
     return int.from_bytes(untrusted_int, "big", signed=False)
 
 
-def read_debug_text(f: IO[bytes], size: int) -> str:
-    """Read arbitrarily long text (for debug purposes), and sanitize it."""
-    untrusted_text = f.read(size).decode("ascii", errors="replace")
+def read_debug_text(text: bytes) -> str:
+    """Read all the buffer and return a sanitized version"""
+    untrusted_text = text.decode("ascii", errors="replace")
     return replace_control_chars(untrusted_text, keep_newlines=True)
 
 
@@ -336,7 +336,7 @@ class IsolationProvider(ABC):
     ) -> Iterator[subprocess.Popen]:
         """Start a conversion process, pass it to the caller, and then clean it up."""
         p = self.start_doc_to_pixels_proc(document)
-        self.start_stderr_thread(p)
+        stderr_thread = self.start_stderr_thread(p)
 
         if platform.system() != "Windows":
             assert os.getpgid(p.pid) != os.getpgid(
@@ -353,17 +353,26 @@ class IsolationProvider(ABC):
                 document, p, timeout_grace=timeout_grace, timeout_force=timeout_force
             )
 
-            if self.should_capture_stderr():
-                self.stderr.seek(0)
-                debug_log = read_debug_text(self.stderr, MAX_CONVERSION_LOG_CHARS)
+            if stderr_thread:
+                # Wait for the thread to complete. If it's still alive, mention it in the debug log.
+                stderr_thread.join(timeout=1)
+
+                debug_bytes = self.stderr.getvalue()
+                debug_log = read_debug_text(debug_bytes)[:MAX_CONVERSION_LOG_CHARS]
+
+                incomplete = "(incomplete)\n" if stderr_thread.is_alive() else ""
+
                 log.info(
                     "Conversion output (doc to pixels)\n"
                     f"{DOC_TO_PIXELS_LOG_START}\n"
                     f"{debug_log}"  # no need for an extra newline here
+                    f"{incomplete}"
                     f"{DOC_TO_PIXELS_LOG_END}"
                 )
 
-    def start_stderr_thread(self, process: subprocess.Popen) -> None:
+    def start_stderr_thread(
+        self, process: subprocess.Popen
+    ) -> Optional[threading.Thread]:
         """Start a thread to read stderr from the process"""
 
         def _stream_stderr(process_stderr: IO[bytes]) -> None:
@@ -380,3 +389,5 @@ class IsolationProvider(ABC):
                 daemon=True,
             )
             stderr_thread.start()
+            return stderr_thread
+        return None

dangerzone/isolation_provider/container.py

@@ -188,12 +188,7 @@ class Container(IsolationProvider):
             + image_name
             + command
         )
-        args = [container_runtime] + args
-        args_str = " ".join(shlex.quote(s) for s in args)
-        log.info("> " + args_str)
-
-        process = self.exec(args)
-        return process
+        return self.exec([container_runtime] + args)
 
     def kill_container(self, name: str) -> None:
         """Terminate a spawned container.