FIXUP: Move process group function at module-level

dangerzone/isolation_provider/base.py

@@ -30,6 +30,38 @@
 TIMEOUT_FORCE = 5
 
 
+def _signal_process_group(p: subprocess.Popen, signo: int) -> None:
+    """Send a signal to a process group."""
+    try:
+        os.killpg(os.getpgid(p.pid), signo)
+    except (ProcessLookupError, PermissionError):
+        # If the process no longer exists, we may encounter the above errors, either
+        # when looking for the process group (ProcessLookupError), or when trying to
+        # kill a process group that no longer exists (PermissionError)
+        return
+    except Exception:
+        log.exception(
+            f"Unexpected error while sending signal {signo} to the"
+            f"document-to-pixels process group (PID: {p.pid})"
+        )
+
+
+def terminate_process_group(p: subprocess.Popen) -> None:
+    """Terminate a process group."""
+    if platform.system() == "Windows":
+        p.terminate()
+    else:
+        _signal_process_group(p, signal.SIGTERM)
+
+
+def kill_process_group(p: subprocess.Popen) -> None:
+    """Forcefully kill a process group."""
+    if platform.system() == "Windows":
+        p.kill()
+    else:
+        _signal_process_group(p, signal.SIGKILL)
+
+
 def read_bytes(f: IO[bytes], size: int, exact: bool = True) -> bytes:
     """Read bytes from a file-like object."""
     buf = f.read(size)
@@ -212,35 +244,6 @@ def terminate_doc_to_pixels_proc(
         """Terminate gracefully the process started for the doc-to-pixels phase."""
         pass
 
-    def _signal_process_group(self, p: subprocess.Popen, signo: int) -> None:
-        """Send a signal to a process group."""
-        try:
-            os.killpg(os.getpgid(p.pid), signo)
-        except (ProcessLookupError, PermissionError):
-            # If the process no longer exists, we may encounter the above errors, either
-            # when looking for the process group (ProcessLookupError), or when trying to
-            # kill a process group that no longer exists (PermissionError)
-            return
-        except Exception:
-            log.exception(
-                f"Unexpected error while sending signal {signo} to the"
-                f"document-to-pixels process group (PID: {p.pid})"
-            )
-
-    def terminate_process_group(self, p: subprocess.Popen) -> None:
-        """Terminate a process group."""
-        if platform.system() == "Windows":
-            p.terminate()
-        else:
-            self._signal_process_group(p, signal.SIGTERM)
-
-    def kill_process_group(self, p: subprocess.Popen) -> None:
-        """Forcefully kill a process group."""
-        if platform.system() == "Windows":
-            p.kill()
-        else:
-            self._signal_process_group(p, signal.SIGKILL)
-
     def ensure_stop_doc_to_pixels_proc(
         self,
         document: Document,
@@ -272,7 +275,7 @@ def ensure_stop_doc_to_pixels_proc(
             )
 
             # Forcefully kill the running process.
-            self.kill_process_group(p)
+            kill_process_group(p)
             try:
                 p.wait(timeout_force)
             except subprocess.TimeoutExpired:

dangerzone/isolation_provider/container.py

@@ -13,7 +13,12 @@
 from ..document import Document
 from ..util import get_tmp_dir  # NOQA : required for mocking in our tests.
 from ..util import get_resource_path, get_subprocess_startupinfo
-from .base import PIXELS_TO_PDF_LOG_END, PIXELS_TO_PDF_LOG_START, IsolationProvider
+from .base import (
+    PIXELS_TO_PDF_LOG_END,
+    PIXELS_TO_PDF_LOG_START,
+    IsolationProvider,
+    terminate_process_group,
+)
 
 TIMEOUT_KILL = 5  # Timeout in seconds until the kill command returns.
 
@@ -436,7 +441,7 @@ def terminate_doc_to_pixels_proc(
         #
         # See also https://github.com/freedomofpress/dangerzone/issues/791
         self.kill_container(self.doc_to_pixels_container_name(document))
-        self.terminate_process_group(p)
+        terminate_process_group(p)
 
     def ensure_stop_doc_to_pixels_proc(  # type: ignore [no-untyped-def]
         self, document: Document, *args, **kwargs

dangerzone/isolation_provider/dummy.py

@@ -8,7 +8,7 @@
 
 from ..document import Document
 from ..util import get_resource_path
-from .base import IsolationProvider
+from .base import IsolationProvider, terminate_process_group
 
 log = logging.getLogger(__name__)
 
@@ -86,7 +86,7 @@ def start_doc_to_pixels_proc(self, document: Document) -> subprocess.Popen:
     def terminate_doc_to_pixels_proc(
         self, document: Document, p: subprocess.Popen
     ) -> None:
-        self.terminate_process_group(p)
+        terminate_process_group(p)
 
     def get_max_parallel_conversions(self) -> int:
         return 1

tests/isolation_provider/base.py

@@ -140,7 +140,7 @@ def test_linger_kill(
         terminate_proc_mock = mocker.patch.object(
             provider, "terminate_doc_to_pixels_proc", return_value=None
         )
-        kill_pg_spy = mocker.spy(provider, "kill_process_group")
+        kill_pg_spy = mocker.spy(base, "kill_process_group")
 
         with provider.doc_to_pixels_proc(doc, timeout_grace=0) as proc:
             pass
@@ -165,8 +165,8 @@ def test_linger_unkillable(
         terminate_proc_mock = mocker.patch.object(
             provider, "terminate_doc_to_pixels_proc", return_value=None
         )
-        kill_pg_mock = mocker.patch.object(
-            provider, "kill_process_group", return_value=None
+        kill_pg_mock = mocker.patch(
+            "dangerzone.isolation_provider.base.kill_process_group", return_value=None
         )
 
         with provider.doc_to_pixels_proc(doc, timeout_grace=0, timeout_force=0) as proc: