isolation_provider: Terminate doc-to-pixels proc

Extend the IsolationProvider class with a
`terminate_doc_to_pixels_proc()` method, which must be implemented by
the Qubes/Container providers and gracefully terminate a process started
for the doc to pixels phase.

Refs #563

dangerzone/isolation_provider/base.py

@@ -195,6 +195,13 @@ class IsolationProvider(ABC):
     def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen:
         pass
 
+    @abstractmethod
+    def terminate_doc_to_pixels_proc(
+        self, document: Document, p: subprocess.Popen
+    ) -> None:
+        """Terminate gracefully the process started for the doc-to-pixels phase."""
+        pass
+
 
 # From global_common:
 

dangerzone/isolation_provider/container.py

@@ -215,6 +215,28 @@ class Container(IsolationProvider):
         args = [container_runtime] + args
         return self.exec(args)
 
+    def kill_container(self, name: str) -> None:
+        """Terminate a spawned container.
+
+        We choose to terminate spawned containers using the `kill` action that the
+        container runtime provides, instead of terminating the process that spawned
+        them. The reason is that this process is not always tied to the underlying
+        container. For instance, in Docker containers, this process is actually
+        connected to the Docker daemon, and killing it will just close the associated
+        standard streams.
+        """
+        container_runtime = self.get_runtime()
+        cmd = [container_runtime, "kill", name]
+        try:
+            subprocess.run(cmd, capture_output=True, check=True)
+        except subprocess.CalledProcessError as e:
+            log.warning(f"Failed to kill container {name}: {str(e)}")
+            log.warning(f"Output from the kill command: {e.output}")
+        except Exception as e:
+            log.exception(
+                f"Unexpected error occurred while killing container {name}: {str(e)}"
+            )
+
     def pixels_to_pdf(
         self, document: Document, tempdir: str, ocr_lang: Optional[str]
     ) -> None:
@@ -273,6 +295,11 @@ class Container(IsolationProvider):
         name = self.doc_to_pixels_container_name(document)
         return self.exec_container(command, name=name)
 
+    def terminate_doc_to_pixels_proc(
+        self, document: Document, p: subprocess.Popen
+    ) -> None:
+        self.kill_container(self.doc_to_pixels_container_name(document))
+
     def get_max_parallel_conversions(self) -> int:
         # FIXME hardcoded 1 until length conversions are better handled
         # https://github.com/freedomofpress/dangerzone/issues/257

dangerzone/isolation_provider/dummy.py

@@ -77,5 +77,10 @@ class Dummy(IsolationProvider):
     def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen:
         return subprocess.Popen("True")
 
+    def terminate_doc_to_pixels_proc(
+        self, document: Document, p: subprocess.Popen
+    ) -> None:
+        pass
+
     def get_max_parallel_conversions(self) -> int:
         return 1

dangerzone/isolation_provider/qubes.py

@@ -77,6 +77,32 @@ class Qubes(IsolationProvider):
 
         return p
 
+    def terminate_doc_to_pixels_proc(
+        self, document: Document, p: subprocess.Popen
+    ) -> None:
+        """Terminate a spawned disposable qube.
+
+        Qubes does not offer a way out of the box to terminate disposable Qubes from
+        domU [1]. Our best bet is to close the standard streams of the process, and hope
+        that the disposable qube will attempt to read/write to them, and thus receive an
+        EOF.
+
+        There are two ways we can do the above; close the standard streams explicitly,
+        or terminate the process. The problem with the latter is that terminating
+        `qrexec-client-vm` happens immediately, and we no longer have a way to learn if
+        the disposable qube actually terminated. That's why we prefer closing the
+        standard streams explicitly, so that we can afterwards use `Popen.wait()` to
+        learn if the qube terminated.
+
+        [1]: https://github.com/freedomofpress/dangerzone/issues/563#issuecomment-2034803232
+        """
+        if p.stdin:
+            p.stdin.close()
+        if p.stdout:
+            p.stdout.close()
+        if p.stderr:
+            p.stderr.close()
+
     def teleport_dz_module(self, wpipe: IO[bytes]) -> None:
         """Send the dangerzone module to another qube, as a zipfile."""
         # Grab the absolute file path of the dangerzone module.