diff --git a/AppImage/scripts/lxc_update_jobs.py b/AppImage/scripts/lxc_update_jobs.py index 1abfce1c..d3bd74a4 100644 --- a/AppImage/scripts/lxc_update_jobs.py +++ b/AppImage/scripts/lxc_update_jobs.py @@ -35,6 +35,9 @@ PARAMS = { RUN_ID = re.compile(r"^[A-Za-z0-9_-]{1,128}$") INTERACTION_ID = re.compile(r"^[A-Za-z0-9_-]{1,64}$") MAX_RUNTIME = 4 * 3600 +# The shared runner restores a guest it started in its EXIT trap. Its package +# observation and `pct shutdown --timeout 60` need time after SIGTERM, too. +STOP_GRACE = 300 RESULT_TTL = 6 * 3600 MAX_OUTPUT = 256 * 1024 MAX_JOBS = 32 @@ -187,7 +190,7 @@ class UpdateJob: break if time.monotonic() - self.started >= MAX_RUNTIME: self._signal("timeout") - if running and self.stop_at and now - self.stop_at >= 2: + if running and self.stop_at and now - self.stop_at >= STOP_GRACE: try: os.killpg(self.process.pid, signal.SIGKILL) except ProcessLookupError: @@ -213,7 +216,7 @@ class UpdateJob: self._append(f"\r\nUpdate viewer failed: {type(exc).__name__}\r\n") self._signal("viewer failure") try: - self.process.wait(timeout=2) + self.process.wait(timeout=STOP_GRACE) except subprocess.TimeoutExpired: os.killpg(self.process.pid, signal.SIGKILL) self.process.wait() diff --git a/tests/test_lxc_update_jobs.py b/tests/test_lxc_update_jobs.py index aea40aeb..26a13473 100644 --- a/tests/test_lxc_update_jobs.py +++ b/tests/test_lxc_update_jobs.py @@ -187,6 +187,31 @@ class JobTests(unittest.TestCase): self.assertEqual(job.exit_code, 124) self.callback.assert_called_once() + def test_cancel_waits_for_the_runners_original_state_cleanup(self): + # apply_updates.sh's EXIT trap may need a 60s package probe followed + # by `pct shutdown --timeout 60`. A two-second kill truncates that trap. + self.script.write_text( + f"trap 'sleep 2.3; echo restored >> {shlex.quote(str(self.events))}; exit 0' TERM\n" + + self.script.read_text()) + job = self.start() + wait_for(lambda: self.events.exists()) + job.cancel() + wait_for(lambda: job.state == 'complete') + self.assertEqual(job.exit_code, 130) + self.assertEqual(self.events.read_text().splitlines(), ['backup', 'restored']) + self.assertNotIn('update', self.events.read_text()) + self.callback.assert_called_once() + self.assertGreater(jobs.STOP_GRACE, 120) + + def test_timeout_still_reaps_a_process_that_ignores_termination(self): + self.script.write_text("trap '' TERM\necho ready\nwhile :; do sleep .02; done\n") + with patch.object(jobs, 'MAX_RUNTIME', .15), patch.object(jobs, 'STOP_GRACE', .1): + job = self.start() + wait_for(lambda: job.state == 'complete') + self.assertEqual(job.exit_code, 124) + self.assertIsNotNone(job.process.poll()) + self.callback.assert_called_once() + def test_output_is_bounded_and_cursor_replay_is_read_only(self): job = self.start() wait_for(lambda: self.events.exists())