fix(ibus): keep engine teardown correct when parent destroy fails (#613)

* fix(ibus): keep engine teardown correct when parent destroy fails

`do_destroy` passed `super().do_destroy` to `_handle_engine_destroy`.
PyGObject binds that vfunc to the GType rather than to the instance, so
calling it with no arguments raises:

    TypeError: IBus.Object.destroy() takes exactly 1 argument (0 given)

This fired on every engine teardown. The exception escaped
`_handle_engine_destroy` before its return, so neither
`_active_instance = ...` nor `_focus_event.clear()` in the caller ran,
leaving a destroyed engine registered as active with focus still set --
the two values `_ibus_ping_status` (#523) reads to decide commit
readiness.

Two changes:

- Bind the parent destroy to the instance, falling back to
  `IBus.Object.destroy(self)` only when the no-arg call raises TypeError,
  so this keeps working if PyGObject binds the vfunc correctly later.
- Guard the parent destroy call so no failure there can skip teardown.

Fixes #606

* refactor(ibus): extract parent-destroy workaround into a testable helper

Codecov flagged the new code as uncovered (36% patch, below the 80%
target). The whole `if IBUS_AVAILABLE:` branch of `do_destroy` is
unreachable from the suite, which only ever exercises that method with
IBUS_AVAILABLE False -- the single line it replaced was already
uncovered on main for the same reason.

Move the workaround into a module-level `_invoke_parent_destroy`, the
same pattern `_handle_engine_destroy` already uses to make `do_destroy`
testable, and bind it with functools.partial at the call site.

Behaviour is unchanged. Coverage of the branch goes from 0 to all but
the one-line partial, and three tests now pin the actual contract:
pass-through when the binding is correct, retry with the instance on the
GType-binding TypeError, and no masking of other exceptions.
This commit is contained in:
Benjamin Eisele
2026-07-29 00:36:24 +05:30
committed by GitHub
parent 161129cf3c
commit 29db3a05a4
2 changed files with 108 additions and 2 deletions
+26 -2
View File
@@ -14,6 +14,7 @@ This is the preferred method for Wayland environments as it works universally
without requiring compositor-specific protocols.
"""
import functools
import logging
import os
import signal
@@ -589,6 +590,23 @@ def restore_xkb_layout(layout: str, variant: str = "", option: str = "") -> bool
return False
def _invoke_parent_destroy(parent_destroy: Callable[[], None], instance: object) -> None:
"""Call the parent engine destroy, working around PyGObject's GType binding.
PyGObject binds the ``do_destroy`` vfunc to the GType rather than to the
instance, so ``super().do_destroy()`` raises
``TypeError: IBus.Object.destroy() takes exactly 1 argument (0 given)``.
Retry with the instance explicitly.
The TypeError path rather than an unconditional ``IBus.Object.destroy``
keeps this working if a future PyGObject binds the vfunc correctly.
"""
try:
parent_destroy()
except TypeError:
IBus.Object.destroy(instance)
def _handle_engine_destroy(
active_instance: Optional["VocalinuxEngine"],
current_instance: object,
@@ -601,7 +619,13 @@ def _handle_engine_destroy(
next_active_instance = None
if ibus_available and super_destroy is not None:
super_destroy()
# The caller assigns our return value to _active_instance and clears the
# focus event, so a failing parent destroy must not skip that teardown:
# otherwise a destroyed engine stays registered as active with focus set.
try:
super_destroy()
except Exception:
logger.exception("Parent engine destroy failed; continuing teardown")
return next_active_instance
@@ -866,7 +890,7 @@ class VocalinuxEngine(IBus.Engine if IBUS_AVAILABLE else object):
logger.debug("VocalinuxEngine instance destroyed")
super_destroy: Optional[Callable[[], None]] = None
if IBUS_AVAILABLE:
super_destroy = super().do_destroy
super_destroy = functools.partial(_invoke_parent_destroy, super().do_destroy, self)
was_active = VocalinuxEngine._active_instance is self
VocalinuxEngine._active_instance = _handle_engine_destroy(
VocalinuxEngine._active_instance,
+82
View File
@@ -2015,6 +2015,88 @@ class TestVocalinuxEngineDestroy(unittest.TestCase):
mock_super_destroy.assert_not_called()
def test_invoke_parent_destroy_passes_through_when_binding_is_correct(self):
"""A parent destroy that accepts no arguments is called as-is (#606)."""
from vocalinux.text_injection.ibus_engine import _invoke_parent_destroy
parent_destroy = MagicMock()
_invoke_parent_destroy(parent_destroy, object())
parent_destroy.assert_called_once_with()
def test_invoke_parent_destroy_retries_with_instance_on_type_error(self):
"""PyGObject binds the vfunc to the GType, so retry with the instance (#606).
``super().do_destroy`` resolves to a method bound to the GType rather
than the engine, so the no-argument call raises TypeError and the real
destroy never happens.
"""
import vocalinux.text_injection.ibus_engine as mod
from vocalinux.text_injection.ibus_engine import _invoke_parent_destroy
parent_destroy = MagicMock(
side_effect=TypeError("IBus.Object.destroy() takes exactly 1 argument (0 given)")
)
instance = object()
fake_ibus = MagicMock()
with patch.object(mod, "IBus", fake_ibus):
_invoke_parent_destroy(parent_destroy, instance)
parent_destroy.assert_called_once_with()
fake_ibus.Object.destroy.assert_called_once_with(instance)
def test_invoke_parent_destroy_propagates_other_errors(self):
"""Only the GType-binding TypeError is worked around; nothing else is masked."""
from vocalinux.text_injection.ibus_engine import _invoke_parent_destroy
parent_destroy = MagicMock(side_effect=RuntimeError("boom"))
with self.assertRaises(RuntimeError):
_invoke_parent_destroy(parent_destroy, object())
def test_do_destroy_clears_active_instance_when_super_raises(self):
"""A failing parent destroy must not skip teardown (#606).
PyGObject binds the destroy vfunc to the GType rather than the instance,
so ``super().do_destroy()`` raised TypeError on every teardown. That
exception escaped before the return value was assigned, leaving a
destroyed engine registered as active with the focus event still set.
"""
from vocalinux.text_injection.ibus_engine import _handle_engine_destroy
engine = object()
failing_super_destroy = MagicMock(
side_effect=TypeError("IBus.Object.destroy() takes exactly 1 argument (0 given)")
)
next_active = _handle_engine_destroy(
active_instance=engine,
current_instance=engine,
ibus_available=True,
super_destroy=failing_super_destroy,
)
failing_super_destroy.assert_called_once()
self.assertIsNone(next_active)
def test_do_destroy_keeps_other_instance_when_super_raises(self):
"""A failing parent destroy still returns the unrelated active engine (#606)."""
from vocalinux.text_injection.ibus_engine import _handle_engine_destroy
active_engine = object()
failing_super_destroy = MagicMock(side_effect=RuntimeError("boom"))
next_active = _handle_engine_destroy(
active_instance=active_engine,
current_instance=object(),
ibus_available=True,
super_destroy=failing_super_destroy,
)
self.assertIs(next_active, active_engine)
def test_do_destroy_handles_missing_super_callback(self):
"""Test destroy handler works when no parent destroy callback is provided."""
from vocalinux.text_injection.ibus_engine import _handle_engine_destroy