From 7a5ddfde38683b5cb151a207d1d6024d0f18a21d Mon Sep 17 00:00:00 2001 From: Jatin K Malik Date: Wed, 5 Aug 2026 21:41:10 -0700 Subject: [PATCH] fix(ibus): restore engine after register_component teardown (#558) (#643) * fix(ibus): restore engine after register_component teardown Stopping the Vocalinux IBus engine process destroys its register_component registration. IBus then runs check_global_engine(), which only searches register_engine_list and treats XML engines like xkb:es::spa as missing, clearing GlobalEngine even when Vocalinux was never selected. Capture a restorable engine before teardown and reselect it afterward so dead keys keep working after quit. Fixes #558 Co-authored-by: Jatin K Malik * fix(ibus): wait for engine exit before restoring GlobalEngine Adversarial review of #643 found that stop_engine_process() returned right after SIGTERM, so switch_engine() could race ahead of IBus check_global_engine() and still leave No global engine. Wait for the process to exit (SIGKILL fallback), retry engine restore, capture a shutdown fallback during prepare_engine, and re-apply XKB after the post-teardown engine switch. Addresses review of #558 / #643. Co-authored-by: Jatin K Malik * fix(ibus): prefer live engine over stale warmup on quit Bugbot on #643: stop() restored the warmup-cached engine even when the user had switched input source afterward, and inject_text refused to refresh that cache once set. Prefer the live non-Vocalinux engine at quit; fall back to the cache only when stuck on vocalinux/unavailable. Always refresh the shutdown fallback from inject_text. Also cover SIGKILL teardown escalation and restore-retry failure paths to lift patch coverage. Co-authored-by: Jatin K Malik --------- Co-authored-by: Cursor Agent Co-authored-by: Jatin K Malik --- src/vocalinux/text_injection/ibus_engine.py | 94 ++++++++-- tests/test_ibus_engine.py | 190 ++++++++++++++++++-- tests/test_ibus_engine_core.py | 8 +- tests/test_ibus_engine_utils.py | 82 +++++++++ 4 files changed, 346 insertions(+), 28 deletions(-) diff --git a/src/vocalinux/text_injection/ibus_engine.py b/src/vocalinux/text_injection/ibus_engine.py index 6054d61..ce071d0 100644 --- a/src/vocalinux/text_injection/ibus_engine.py +++ b/src/vocalinux/text_injection/ibus_engine.py @@ -839,8 +839,13 @@ def start_engine_process() -> bool: return False -def stop_engine_process() -> None: - """Stop the IBus engine process if running.""" +def stop_engine_process(wait_timeout: float = 2.0) -> None: + """Stop the IBus engine process if running. + + Waits for the process to actually exit so callers that restore the user's + IBus engine afterward run after ``register_component`` teardown has been + observed by ibus-daemon (issue #558). + """ try: if not PID_FILE.exists(): logger.debug("No PID file found, engine not running") @@ -857,6 +862,29 @@ def stop_engine_process() -> None: return os.kill(pid, signal.SIGTERM) + # Wait for exit: IBus clears/repairs GlobalEngine on component destroy, + # which only runs after the process actually disconnects (#558). + deadline = time.monotonic() + max(wait_timeout, 0.0) + while time.monotonic() < deadline: + try: + os.kill(pid, 0) + except OSError: + break + time.sleep(0.05) + else: + # Still alive — escalate so teardown (and GlobalEngine repair) finishes. + try: + os.kill(pid, signal.SIGKILL) + except OSError: + pass + kill_deadline = time.monotonic() + 0.5 + while time.monotonic() < kill_deadline: + try: + os.kill(pid, 0) + except OSError: + break + time.sleep(0.05) + logger.info(f"IBus engine process (PID {pid}) stopped") PID_FILE.unlink() except (OSError, ValueError, FileNotFoundError) as e: @@ -1206,7 +1234,18 @@ class IBusTextInjector: The Vocalinux engine is only needed while committing dictated text. Keeping the user's normal IBus/XKB engine active preserves dead-key composition and layout-specific behavior during ordinary typing. + + Captures the current engine for shutdown restore only. Injection still + resolves the restore target at commit time so a stale warmup value is not + used mid-session (see #623); shutdown needs a fallback when quit leaves + Vocalinux selected after a failed inject restore (#558). """ + if not is_engine_active(): + captured = get_current_engine() + if captured and captured != ENGINE_NAME: + self._previous_engine = captured + logger.debug(f"Captured current engine for shutdown restore: {captured}") + if not start_engine_process(): raise IBusSetupError("Failed to start IBus engine process. Check logs for details.") @@ -1303,18 +1342,49 @@ class IBusTextInjector: captured before injection (e.g. on GNOME/Wayland with no global IBus engine), restoration is skipped to avoid switching the user to a wrong layout (see issue #497). + + Engine restoration must run *after* ``stop_engine_process()`` has + observed process exit. Killing the process that called + ``register_component`` makes IBus run ``check_global_engine()``, which + only searches ``register_engine_list`` and treats XML engines such as + ``xkb:es::spa`` as missing, clearing the global engine even when + Vocalinux was never selected (issue #558). + + Prefer the *live* engine at quit when it is a real non-Vocalinux + engine, so a user who changed input source after warmup is not + switched back to a stale cache. Fall back to ``_previous_engine`` only + when the live engine is Vocalinux or unavailable (failed inject restore). """ - if self._previous_engine: - logger.info(f"Restoring previous engine: {self._previous_engine}") - switch_engine(self._previous_engine) - self._previous_engine = None + current = get_current_engine() + if current and current != ENGINE_NAME: + restore_engine = current + else: + restore_engine = self._previous_engine + if restore_engine == ENGINE_NAME: + restore_engine = None + + # Stop the engine process first and wait for exit — teardown can clear + # IBus GlobalEngine (#558). Restore must land after that check. + stop_engine_process() + + if restore_engine: + logger.info(f"Restoring previous engine after teardown: {restore_engine}") + restored = False + for attempt in range(3): + if switch_engine(restore_engine): + restored = True + break + time.sleep(0.15 * (attempt + 1)) + if not restored: + logger.error(f"Failed to restore IBus engine after teardown: {restore_engine}") else: logger.info( - "Skipping engine restoration — " "no valid engine was captured before injection" + "Skipping engine restoration — no restorable engine was available at shutdown" ) + self._previous_engine = None - # Restore the XKB layout that was captured during setup - # This ensures the user's original keyboard layout is preserved + # XKB last: switching engines can override layout (#292). Re-apply after + # the post-teardown engine restore so we don't lose the user's map. if self._previous_xkb_layout: layout, variant, option = self._previous_xkb_layout if layout: @@ -1322,9 +1392,6 @@ class IBusTextInjector: restore_xkb_layout(layout, variant, option) self._previous_xkb_layout = None - # Stop the engine process - stop_engine_process() - def inject_text(self, text: str) -> bool: """ Inject text using the IBus engine. @@ -1352,6 +1419,9 @@ class IBusTextInjector: return False restore_engine = current_engine + # Always refresh the shutdown fallback to the live engine so a later + # quit does not restore a stale warmup value (Bugbot on #643 / #558). + self._previous_engine = current_engine logger.debug( f"Temporarily activating Vocalinux IBus engine (will restore {current_engine})" ) diff --git a/tests/test_ibus_engine.py b/tests/test_ibus_engine.py index 8d04e64..82077d4 100644 --- a/tests/test_ibus_engine.py +++ b/tests/test_ibus_engine.py @@ -658,24 +658,85 @@ class TestIBusTextInjector(unittest.TestCase): with self.assertRaises(RuntimeError): IBusTextInjector() + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None) @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") - @patch("vocalinux.text_injection.ibus_engine.switch_engine") - def test_stop_restores_engine(self, mock_switch, mock_ensure_dir): - """Test stop() restores previous engine.""" + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) + def test_stop_restores_engine( + self, mock_switch, mock_ensure_dir, mock_get_current, mock_stop_proc + ): + """Test stop() restores cached engine when live engine is unavailable (#558).""" from vocalinux.text_injection.ibus_engine import IBusTextInjector + parent = MagicMock() + parent.attach_mock(mock_stop_proc, "stop_proc") + parent.attach_mock(mock_switch, "switch") + injector = IBusTextInjector(auto_activate=False) injector._previous_engine = "xkb:fr::fra" injector.stop() + mock_stop_proc.assert_called_once() mock_switch.assert_called_once_with("xkb:fr::fra") + self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"]) self.assertIsNone(injector._previous_engine) + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:de::deu") + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") - def test_stop_no_previous_engine(self, mock_ensure_dir): + def test_stop_prefers_live_engine_over_stale_warmup_cache( + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc + ): + """Quit must restore the live input source, not a stale warmup cache (#643 Bugbot).""" + from vocalinux.text_injection.ibus_engine import IBusTextInjector + + injector = IBusTextInjector(auto_activate=False) + injector._previous_engine = "xkb:es::spa" + + injector.stop() + + mock_get_current.assert_called() + mock_stop_proc.assert_called_once() + mock_switch.assert_called_once_with("xkb:de::deu") + self.assertIsNone(injector._previous_engine) + + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa") + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_stop_restores_current_engine_when_previous_unset( + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc + ): + """Warmup quit must restore the live engine after register_component teardown (#558).""" + from vocalinux.text_injection.ibus_engine import IBusTextInjector + + parent = MagicMock() + parent.attach_mock(mock_stop_proc, "stop_proc") + parent.attach_mock(mock_switch, "switch") + + injector = IBusTextInjector(auto_activate=False) + self.assertIsNone(injector._previous_engine) + + injector.stop() + + mock_get_current.assert_called() + mock_stop_proc.assert_called_once() + mock_switch.assert_called_once_with("xkb:es::spa") + self.assertEqual([c[0] for c in parent.mock_calls], ["stop_proc", "switch"]) + + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None) + @patch("vocalinux.text_injection.ibus_engine.switch_engine") + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_stop_no_previous_engine( + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc + ): """Test stop() when no previous engine was saved.""" from vocalinux.text_injection.ibus_engine import IBusTextInjector @@ -683,33 +744,49 @@ class TestIBusTextInjector(unittest.TestCase): # Should not raise injector.stop() + mock_stop_proc.assert_called_once() + mock_switch.assert_not_called() @patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout") @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") - @patch("vocalinux.text_injection.ibus_engine.switch_engine") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa") + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") def test_stop_restores_xkb_layout( - self, mock_ensure_dir, mock_switch, mock_stop_proc, mock_restore_xkb + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_restore_xkb ): - """Test stop() restores the captured XKB layout (#292).""" + """Test stop() restores XKB after post-teardown engine restore (#292/#558).""" from vocalinux.text_injection.ibus_engine import IBusTextInjector + parent = MagicMock() + parent.attach_mock(mock_stop_proc, "stop_proc") + parent.attach_mock(mock_switch, "switch") + parent.attach_mock(mock_restore_xkb, "restore_xkb") + injector = IBusTextInjector(auto_activate=False) + injector._previous_engine = "xkb:fr::fra" # stale; live engine should win injector._previous_xkb_layout = ("es", "catalan", "compose:menu") injector.stop() mock_restore_xkb.assert_called_once_with("es", "catalan", "compose:menu") + mock_stop_proc.assert_called_once() + mock_switch.assert_called_once_with("xkb:es::spa") + self.assertEqual( + [c[0] for c in parent.mock_calls], + ["stop_proc", "switch", "restore_xkb"], + ) self.assertIsNone(injector._previous_xkb_layout) @patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout") @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value=None) @patch("vocalinux.text_injection.ibus_engine.switch_engine") @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") def test_stop_skips_xkb_restore_when_no_layout( - self, mock_ensure_dir, mock_switch, mock_stop_proc, mock_restore_xkb + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc, mock_restore_xkb ): """Test stop() skips XKB restore when no layout was captured.""" from vocalinux.text_injection.ibus_engine import IBusTextInjector @@ -720,6 +797,50 @@ class TestIBusTextInjector(unittest.TestCase): injector.stop() mock_restore_xkb.assert_not_called() + mock_switch.assert_not_called() + + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch( + "vocalinux.text_injection.ibus_engine.get_current_engine", + return_value="vocalinux", + ) + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_stop_uses_cached_engine_when_stuck_on_vocalinux( + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc + ): + """If quit finds vocalinux selected, fall back to the warmup-captured engine (#558).""" + from vocalinux.text_injection.ibus_engine import IBusTextInjector + + injector = IBusTextInjector(auto_activate=False) + injector._previous_engine = "xkb:es::spa" + + injector.stop() + + mock_get_current.assert_called() + mock_stop_proc.assert_called_once() + mock_switch.assert_called_once_with("xkb:es::spa") + + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch( + "vocalinux.text_injection.ibus_engine.get_current_engine", + return_value="vocalinux", + ) + @patch("vocalinux.text_injection.ibus_engine.switch_engine") + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_stop_skips_restore_when_only_vocalinux_active( + self, mock_ensure_dir, mock_switch, mock_get_current, mock_stop_proc + ): + """Do not try to reselect vocalinux after its process has been torn down.""" + from vocalinux.text_injection.ibus_engine import IBusTextInjector + + injector = IBusTextInjector(auto_activate=False) + injector.stop() + + mock_stop_proc.assert_called_once() + mock_switch.assert_not_called() @patch("vocalinux.text_injection.ibus_engine.restore_xkb_layout") @patch("vocalinux.text_injection.ibus_engine.get_current_xkb_layout") @@ -757,6 +878,8 @@ class TestIBusTextInjector(unittest.TestCase): self.assertEqual(injector._previous_xkb_layout, ("fr", "azerty", "")) mock_restore_xkb.assert_called_once_with("fr", "azerty", "") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:fr::fra") + @patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False) @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") @patch("vocalinux.text_injection.ibus_engine.start_engine_process", return_value=True) @@ -768,6 +891,8 @@ class TestIBusTextInjector(unittest.TestCase): mock_wait_ready, mock_start_engine, mock_ensure_dir, + mock_is_active, + mock_get_current, ): """Test warmup starts the process without replacing the user's IBus engine.""" from vocalinux.text_injection.ibus_engine import IBusTextInjector @@ -779,13 +904,13 @@ class TestIBusTextInjector(unittest.TestCase): mock_wait_ready.assert_called_once_with(require_active=False) mock_switch.assert_not_called() - @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:fr::fra") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa") @patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False) @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") @patch("vocalinux.text_injection.ibus_engine.start_engine_process", return_value=True) @patch("vocalinux.text_injection.ibus_engine.IBusTextInjector._wait_for_engine_ready") - def test_prepare_engine_does_not_cache_restore_engine( + def test_prepare_engine_caches_shutdown_restore_engine( self, mock_wait_ready, mock_start_engine, @@ -793,15 +918,15 @@ class TestIBusTextInjector(unittest.TestCase): mock_is_active, mock_get_current, ): - """Test warmup does not cache an engine that can become stale before injection.""" + """Warmup caches the live engine so quit can restore after a stuck vocalinux (#558).""" from vocalinux.text_injection.ibus_engine import IBusTextInjector injector = IBusTextInjector(auto_activate=False) injector.prepare_engine() - self.assertIsNone(injector._previous_engine) - mock_get_current.assert_not_called() - mock_is_active.assert_not_called() + self.assertEqual(injector._previous_engine, "xkb:es::spa") + mock_get_current.assert_called() + mock_is_active.assert_called() @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="libpinyin") @patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False) @@ -834,11 +959,48 @@ class TestIBusTextInjector(unittest.TestCase): self.assertTrue(result) mock_sock.sendall.assert_called_once_with("Bonjour".encode("utf-8")) + self.assertEqual(injector._previous_engine, "libpinyin") self.assertEqual( [call.args[0] for call in mock_switch.call_args_list], [ENGINE_NAME, "libpinyin"], ) + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:de::deu") + @patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False) + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=True) + @patch("socket.socket") + @patch("vocalinux.text_injection.ibus_engine.SOCKET_PATH") + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_inject_text_refreshes_stale_shutdown_fallback( + self, + mock_ensure_dir, + mock_socket_path, + mock_socket_cls, + mock_switch, + mock_is_active, + mock_get_current, + ): + """inject_text overwrites a stale warmup cache with the live engine (#643 Bugbot).""" + from vocalinux.text_injection.ibus_engine import ENGINE_NAME, IBusTextInjector + + mock_socket_path.exists.return_value = True + mock_sock = MagicMock() + mock_sock.__enter__.return_value = mock_sock + mock_sock.__exit__.return_value = None + mock_sock.recv.return_value = b"OK" + mock_socket_cls.return_value = mock_sock + + injector = IBusTextInjector(auto_activate=False) + injector._previous_engine = "xkb:es::spa" + + self.assertTrue(injector.inject_text("Hallo")) + self.assertEqual(injector._previous_engine, "xkb:de::deu") + self.assertEqual( + [call.args[0] for call in mock_switch.call_args_list], + [ENGINE_NAME, "xkb:de::deu"], + ) + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="libpinyin") @patch("vocalinux.text_injection.ibus_engine.is_engine_active", return_value=False) @patch("vocalinux.text_injection.ibus_engine.switch_engine", side_effect=[True, False]) diff --git a/tests/test_ibus_engine_core.py b/tests/test_ibus_engine_core.py index c75a9df..577c971 100644 --- a/tests/test_ibus_engine_core.py +++ b/tests/test_ibus_engine_core.py @@ -574,16 +574,20 @@ class TestIBusTextInjector(unittest.TestCase): self.assertIsNotNone(injector) @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") @patch("vocalinux.text_injection.ibus_engine.switch_engine") @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="other-engine") - def test_ibus_text_injector_stop(self, mock_current_engine, mock_switch, mock_ensure_dir): - """Test IBusTextInjector stop restores previous engine.""" + def test_ibus_text_injector_stop( + self, mock_current_engine, mock_switch, mock_stop_proc, mock_ensure_dir + ): + """Test IBusTextInjector stop restores previous engine after teardown.""" from vocalinux.text_injection.ibus_engine import IBusTextInjector injector = IBusTextInjector(auto_activate=False) injector._previous_engine = "other-engine" injector.stop() + mock_stop_proc.assert_called_once() mock_switch.assert_called_once_with("other-engine") @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") diff --git a/tests/test_ibus_engine_utils.py b/tests/test_ibus_engine_utils.py index e7c4e46..20c7e43 100644 --- a/tests/test_ibus_engine_utils.py +++ b/tests/test_ibus_engine_utils.py @@ -4,6 +4,7 @@ Final comprehensive tests for IBus engine. These tests exercise code paths to improve coverage without platform-specific issues. """ +import signal import subprocess import sys import unittest @@ -319,6 +320,87 @@ class TestStopEngineProcess(unittest.TestCase): # Verify the function handles missing PID file gracefully self.assertIsNone(result) + @patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None) + @patch("vocalinux.text_injection.ibus_engine.os.kill") + @patch("vocalinux.text_injection.ibus_engine.Path") + @patch("vocalinux.text_injection.ibus_engine.PID_FILE") + def test_stop_engine_process_waits_for_exit( + self, mock_pid_file, mock_path_cls, mock_kill, mock_sleep + ): + """SIGTERM path must wait until the process is gone before returning (#558).""" + from vocalinux.text_injection.ibus_engine import stop_engine_process + + mock_pid_file.exists.return_value = True + mock_pid_file.read_text.return_value = "4242\n" + + cmdline = MagicMock() + cmdline.exists.return_value = True + cmdline.read_text.return_value = "python /x/vocalinux/text_injection/ibus_engine.py --ibus" + mock_path_cls.return_value = cmdline + + # SIGTERM succeeds; first liveness check still alive; second gone. + mock_kill.side_effect = [None, None, OSError(3, "No such process")] + + stop_engine_process(wait_timeout=1.0) + + self.assertEqual(mock_kill.call_args_list[0][0], (4242, signal.SIGTERM)) + self.assertGreaterEqual(mock_kill.call_count, 3) + mock_pid_file.unlink.assert_called() + mock_sleep.assert_called() + + @patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None) + @patch("vocalinux.text_injection.ibus_engine.os.kill") + @patch("vocalinux.text_injection.ibus_engine.Path") + @patch("vocalinux.text_injection.ibus_engine.PID_FILE") + def test_stop_engine_process_sigkills_when_term_ignored( + self, mock_pid_file, mock_path_cls, mock_kill, mock_sleep + ): + """Escalate to SIGKILL if the engine ignores SIGTERM (#558 wait path).""" + from vocalinux.text_injection.ibus_engine import stop_engine_process + + mock_pid_file.exists.return_value = True + mock_pid_file.read_text.return_value = "4242\n" + + cmdline = MagicMock() + cmdline.exists.return_value = True + cmdline.read_text.return_value = "python /x/vocalinux/text_injection/ibus_engine.py --ibus" + mock_path_cls.return_value = cmdline + + # SIGTERM, then keep reporting alive until wait timeout, then SIGKILL succeeds. + mock_kill.side_effect = [None, None, None, OSError(3, "No such process")] + with patch( + "vocalinux.text_injection.ibus_engine.time.monotonic", + side_effect=[0.0, 0.0, 2.1, 2.1, 2.2, 2.7], + ): + stop_engine_process(wait_timeout=2.0) + + self.assertEqual(mock_kill.call_args_list[0][0], (4242, signal.SIGTERM)) + self.assertTrue(any(c[0] == (4242, signal.SIGKILL) for c in mock_kill.call_args_list)) + mock_pid_file.unlink.assert_called() + + +class TestStopRestoreRetry(unittest.TestCase): + """Coverage for stop() restore retry after teardown.""" + + @patch("vocalinux.text_injection.ibus_engine.stop_engine_process") + @patch("vocalinux.text_injection.ibus_engine.get_current_engine", return_value="xkb:es::spa") + @patch("vocalinux.text_injection.ibus_engine.switch_engine", return_value=False) + @patch("vocalinux.text_injection.ibus_engine.time.sleep", return_value=None) + @patch("vocalinux.text_injection.ibus_engine.IBUS_AVAILABLE", True) + @patch("vocalinux.text_injection.ibus_engine.ensure_ibus_dir") + def test_stop_retries_failed_engine_restore( + self, mock_ensure_dir, mock_sleep, mock_switch, mock_get_current, mock_stop_proc + ): + """stop() retries switch_engine after teardown when the first attempts fail.""" + from vocalinux.text_injection.ibus_engine import IBusTextInjector + + injector = IBusTextInjector(auto_activate=False) + injector.stop() + + self.assertEqual(mock_switch.call_count, 3) + mock_switch.assert_called_with("xkb:es::spa") + self.assertEqual(mock_sleep.call_count, 3) + if __name__ == "__main__": unittest.main()