pdf-converter: stop DispVM conversion when GUI cancel is clicked

Cancel in the Zenity progress dialog did not stop the running conversion job.\nThe pipeline wrapper could exit while qvm-convert-pdf (and qrexec subprocesses)\nkept running in the background.\n\nFix this in two places:\n- qvm-convert-pdf.gnome now tracks converter PID and sends SIGTERM when\n  Zenity returns non-zero (Cancel).\n- client cleanup now terminates an active qrexec process on CancelledError,\n  handles SIGTERM the same way as SIGINT, and fixes the termination guard\n  to stop running processes on failures.\n\nAlso add async unit tests for cancellation and signal registration paths.\n\nFixes QubesOS/qubes-issues#10274

Author: Jayant-kernel

qubespdfconverter/client.py

@@ -491,11 +491,13 @@ async def run(self, archive, depth, in_place):
 
                 self.bar.set_job_status(Status.FAIL)
                 await ERROR_LOGS.put(f"{self.path.name}: {e}")
-                if self.proc.returncode is not None:
+                if self.proc.returncode is None:
                     await terminate_proc(self.proc)
                 raise
             except asyncio.CancelledError:
                 self.bar.set_job_status(Status.CANCELLED)
+                if self.proc.returncode is None:
+                    await terminate_proc(self.proc)
                 raise
 
         self.bar.set_job_status(Status.DONE)
@@ -599,10 +601,12 @@ async def run(params):
                                                  params["batch"],
                                                  params["in_place"])))
 
-    asyncio.get_running_loop().add_signal_handler(
-        signal.SIGINT,
-        lambda: asyncio.ensure_future(sigint_handler(tasks))
-    )
+    loop = asyncio.get_running_loop()
+    for sig in (signal.SIGINT, signal.SIGTERM):
+        loop.add_signal_handler(
+            sig,
+            lambda: asyncio.ensure_future(sigint_handler(tasks))
+        )
 
     results = await asyncio.gather(*tasks, return_exceptions=True)
     completed = results.count(None)

qubespdfconverter/test_client.py

@@ -0,0 +1,73 @@
+#!/usr/bin/python3
+
+import asyncio
+import signal
+import unittest
+from pathlib import Path
+from unittest import mock
+
+from qubespdfconverter.client import Job, PageError, run
+
+
+class DummyProc:
+    def __init__(self):
+        self.returncode = None
+        self.terminated = False
+        self.stdin = mock.Mock()
+        self.stdout = mock.Mock()
+
+    def terminate(self):
+        self.terminated = True
+        self.returncode = -signal.SIGTERM
+
+    async def wait(self):
+        if self.returncode is None:
+            self.returncode = 0
+
+
+class TC_ClientCancel(unittest.IsolatedAsyncioTestCase):
+    async def test_000_cancel_terminates_qrexec_proc(self):
+        job = Job(Path("/tmp/test.pdf"), 0)
+        proc = DummyProc()
+
+        with mock.patch("asyncio.create_subprocess_exec", new=mock.AsyncMock(return_value=proc)):
+            job._setup = mock.AsyncMock()
+            job._start = mock.AsyncMock(side_effect=asyncio.CancelledError)
+
+            with self.assertRaises(asyncio.CancelledError):
+                await job.run(Path("/tmp/archive"), depth=1, in_place=False)
+
+        self.assertTrue(proc.terminated)
+
+    async def test_001_failure_terminates_qrexec_proc(self):
+        job = Job(Path("/tmp/test.pdf"), 0)
+        proc = DummyProc()
+
+        with mock.patch("asyncio.create_subprocess_exec", new=mock.AsyncMock(return_value=proc)):
+            job._setup = mock.AsyncMock(side_effect=PageError)
+
+            with self.assertRaises(PageError):
+                await job.run(Path("/tmp/archive"), depth=1, in_place=False)
+
+        self.assertTrue(proc.terminated)
+
+    async def test_002_register_sigterm_handler(self):
+        loop = asyncio.get_running_loop()
+        add_handler_mock = mock.Mock()
+
+        with mock.patch.object(loop, "add_signal_handler", new=add_handler_mock):
+            result = await run({
+                "resolution": 300,
+                "files": [],
+                "archive": Path("/tmp/archive"),
+                "batch": 1,
+                "in_place": False,
+            })
+
+        self.assertFalse(result)
+        handled = {call.args[0] for call in add_handler_mock.mock_calls}
+        self.assertEqual(handled, {signal.SIGINT, signal.SIGTERM})
+
+
+if __name__ == "__main__":
+    unittest.main()

qvm-convert-pdf.gnome

@@ -26,4 +26,21 @@ fi
 
 export PROGRESS_FOR_GUI="yes"
 
-/usr/bin/qvm-convert-pdf "$@" | zenity --progress --text="Converting PDF using Disposable VM..." --auto-close --auto-kill
+fifo="$(mktemp -u)"
+mkfifo "$fifo" || exit 1
+
+/usr/bin/qvm-convert-pdf "$@" >"$fifo" &
+converter_pid="$!"
+
+zenity --progress --text="Converting PDF using Disposable VM..." --auto-close <"$fifo"
+zenity_rc="$?"
+
+rm -f "$fifo"
+
+if [ "$zenity_rc" -ne 0 ]; then
+    kill -TERM "$converter_pid" 2>/dev/null
+    wait "$converter_pid" 2>/dev/null
+    exit 1
+fi
+
+wait "$converter_pid"