From dc718d3e29ec0812ceffd9b111d07342fbec5ee6 Mon Sep 17 00:00:00 2001 From: Paul Kilar Date: Sat, 29 Aug 2026 20:24:00 -0400 Subject: [PATCH] ePassport: fix the segfault on startup The app died with SIGSEGV before drawing anything, on every entry point including --help. The window minimum size was pre-seeded into Kivy's Config, and WindowBase.__init__ is the only place Kivy reads it - so it was applied from inside WindowSDL.create_window(), while Window.initialized was still False. If the window SDL has just made is smaller than that minimum, SDL resizes it on the spot; the resize comes back through the SDL event filter into EventLoop.idle(), which runs the clock and re-enters create_window(). Still not initialised, that call runs setup_window() again and resizes again, until the C stack gives out. A scaled HiDPI session is enough to reach it: with sdl2-compat on SDL3 under Wayland sizes come back in logical points, so the inherited 800x600 default arrives as a 500x375 window, under the 760x520 minimum. Set the minimum on Window once it exists instead, where the same resize takes Kivy's cheap already-initialised path. That exposed a second fault on the same path. _install_kivy_logging attached a bare StreamHandler() to Kivy's own logger, and kivy.logger replaces sys.stderr with a stream that feeds whatever is written to it back in as a warning - kivy/logger.py cautions about exactly this. Any warning _KivyNoise did not drop answered itself until the recursion limit stopped it, with stderr as the broken part, so nothing legible came out. Not theoretical: this machine logs "MTD: Unable to open device" at startup, which would have killed the app as soon as the segfault was out of the way. Point the handler at the real stderr, which Kivy leaves alone. Behaviour change: a window that opens smaller than 760x520 is now grown to it, rather than the constraint being imposed while the window is built. Testing: 257 passed, 2 skipped (was 254/2); --help, a plain launch, --dump on a generated sample and -v all start and stay up; black clean. Python-only change, so no client or firmware build matrix applies. Co-Authored-By: Claude Opus 5 --- tools/ePassport/epassport/app.py | 45 +++++++++++------ tools/ePassport/tests/test_startup.py | 69 +++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 tools/ePassport/tests/test_startup.py diff --git a/tools/ePassport/epassport/app.py b/tools/ePassport/epassport/app.py index f27df9a25..03a87ce96 100644 --- a/tools/ePassport/epassport/app.py +++ b/tools/ePassport/epassport/app.py @@ -32,17 +32,6 @@ if not _VERBOSE: os.environ.setdefault("OPENCV_LOG_LEVEL", "DEBUG" if _VERBOSE else "ERROR") os.environ.setdefault("OPENCV_VIDEOIO_DEBUG", "1" if _VERBOSE else "0") -from kivy.config import Config # noqa: E402 - -# Set the minimum size here rather than on Window: assigning Window.minimum_* -# warns while the other half of the pair is still zero, whichever order it is -# done in. -# Screen space is worth being careful with: the passport page scales as a -# unit, so the window can go a long way down before anything stops being -# readable. -Config.set("graphics", "minimum_width", "760") -Config.set("graphics", "minimum_height", "520") - from kivy.app import App # noqa: E402 from kivy.clock import Clock # noqa: E402 from kivy.core.window import Window # noqa: E402 @@ -70,11 +59,29 @@ log = logging.getLogger("epassport") #: What the window manager shows. APP_TITLE = "ePassport" +#: The smallest window the passport page stays readable in. +MIN_WINDOW_SIZE = (760, 520) + #: How many files a full dump writes, for the progress bar's denominator when #: EF_COM has not been read yet. DEFAULT_EXPECTED_FILES = 9 +def apply_minimum_window_size(window) -> None: + """Constrain the window, once it exists. + + Setting this through Config instead makes Kivy apply it inside + create_window(), where the resize recurses until the stack gives out. + """ + minimum_width, minimum_height = MIN_WINDOW_SIZE + window.size = ( + max(window.width, minimum_width), + max(window.height, minimum_height), + ) + window.minimum_width = minimum_width + window.minimum_height = minimum_height + + class Root(BoxLayout): """The window: screen manager plus the status bar.""" @@ -139,8 +146,10 @@ class Pm3PassportApp(App): def _restore_window_size(self) -> None: """Reopen at the size the user last left, not a size we insist on.""" width, height = self.settings.window_width, self.settings.window_height - if width >= 760 and height >= 520: + minimum_width, minimum_height = MIN_WINDOW_SIZE + if width >= minimum_width and height >= minimum_height: Window.size = (width, height) + apply_minimum_window_size(Window) Window.bind(on_resize=self._remember_window_size) def _remember_window_size(self, _window, width: int, height: int) -> None: @@ -639,7 +648,11 @@ class _KivyNoise(logging.Filter): something the app never uses. """ - NOISE = ("Cutbuffer", "Unable to find any valuable Cutbuffer provider") + NOISE = ( + "Cutbuffer", + "Unable to find any valuable Cutbuffer provider", + "Both Window.minimum_width and Window.minimum_height", + ) def filter(self, record: logging.LogRecord) -> bool: message = record.getMessage() @@ -652,7 +665,11 @@ def _install_kivy_logging(verbose: bool) -> None: if verbose: return # Kivy kept its own console handler - handler = logging.StreamHandler() + if sys.__stderr__ is None: + return # pythonw and some frozen builds have no stderr + # Not sys.stderr: Kivy replaces it with a stream that feeds writes back + # in as warnings, so a handler on its own logger would loop. + handler = logging.StreamHandler(sys.__stderr__) handler.setFormatter(logging.Formatter("%(levelname)s kivy: %(message)s")) handler.addFilter(_KivyNoise()) Logger.addHandler(handler) diff --git a/tools/ePassport/tests/test_startup.py b/tools/ePassport/tests/test_startup.py new file mode 100644 index 000000000..fbcd3f9cf --- /dev/null +++ b/tools/ePassport/tests/test_startup.py @@ -0,0 +1,69 @@ +"""Startup checks. These run in a subprocess: importing the app builds the +Kivy window, so a segfault would take the test session down with it.""" + +from __future__ import annotations + +import os +import re +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +LAUNCHER = ROOT / "ePassport.py" + + +@pytest.mark.skipif( + not (os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY")), + reason="starting the app needs a display", +) +def test_the_launcher_starts() -> None: + """--help must not die on a signal.""" + proc = subprocess.run( + [sys.executable, str(LAUNCHER), "--help"], + capture_output=True, + text=True, + timeout=180, + env={**os.environ, "KIVY_NO_CONSOLELOG": "1"}, + ) + how = ( + f"killed by signal {-proc.returncode}" + if proc.returncode < 0 + else f"exited {proc.returncode}" + ) + assert proc.returncode == 0, f"--help {how}\n{proc.stderr[-2000:]}" + assert "--dump" in proc.stdout + + +@pytest.mark.skipif( + not (os.environ.get("DISPLAY") or os.environ.get("WAYLAND_DISPLAY")), + reason="importing the app builds a window, which needs a display", +) +def test_a_kivy_warning_does_not_take_the_app_down_with_it() -> None: + """A Kivy warning must not loop through the stderr Kivy replaced.""" + proc = subprocess.run( + [ + sys.executable, + "-c", + "import sys; sys.path.insert(0, %r)\n" % str(ROOT) + + "from epassport.app import _install_kivy_logging\n" + + "from kivy.logger import Logger\n" + + "_install_kivy_logging(False)\n" + + "Logger.warning('an ordinary kivy warning')\n", + ], + capture_output=True, + text=True, + timeout=180, + ) + assert proc.returncode == 0, f"exited {proc.returncode}\n{proc.stderr[-2000:]}" + assert "RecursionError" not in proc.stderr + + +def test_the_window_minimum_is_not_pre_seeded_into_kivy_config() -> None: + """Read the source: importing the app to check is what crashed.""" + source = (ROOT / "epassport" / "app.py").read_text() + assert not re.search( + r"""Config\.set\(\s*['"]graphics['"]\s*,\s*['"]minimum_""", source + ), "the minimum size must be set on Window, not pre-seeded into Config"