From 0d9f98561464712170d8bad94ad5c8746270a9a8 Mon Sep 17 00:00:00 2001 From: Dundy Pasupuleti Date: Sat, 12 Sep 2026 20:58:25 +0000 Subject: [PATCH 1/2] fix AppLauncher: leave SIGSEGV/SIGABRT at SIG_DFL Stop registering a Python handler for SIGABRT so fatal signals keep the default disposition, terminate cleanly, and can produce core dumps. SIGSEGV was already left unhandled; graceful teardown remains on SIGTERM. Fixes #7774 Signed-off-by: Dundy Pasupuleti --- CONTRIBUTORS.md | 1 + .../fix-applauncher-fatal-signal-defaults.rst | 6 ++++ source/isaaclab/isaaclab/app/app_launcher.py | 21 ++++++------- .../test/app/test_simulation_app_lifecycle.py | 30 +++++++++++++++++++ 4 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 17e5dcd01062..f6d454300424 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -86,6 +86,7 @@ Guidelines for modifications: * Diego Ferigo * Dongxuan Fan * Dorsa Rohani +* Dundy Pasupuleti * Ege Sekkin * Emily Sturman * Emmanuel Ferdman diff --git a/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst b/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst new file mode 100644 index 000000000000..2caf6e4379f0 --- /dev/null +++ b/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst @@ -0,0 +1,6 @@ +Fixed +^^^^^ + +* Fixed :class:`~isaaclab.app.AppLauncher` registering a Python handler for + ``SIGABRT``. fatal signals stay at ``SIG_DFL`` so crashes terminate and can + produce core dumps; graceful shutdown remains on ``SIGTERM`` only. diff --git a/source/isaaclab/isaaclab/app/app_launcher.py b/source/isaaclab/isaaclab/app/app_launcher.py index 8bc108266c54..2cf3547c39dc 100644 --- a/source/isaaclab/isaaclab/app/app_launcher.py +++ b/source/isaaclab/isaaclab/app/app_launcher.py @@ -1535,15 +1535,15 @@ class _SimulationAppLifecycle: * Normal exit: the ``atexit`` hook closes the app once; a pending unhandled exception turns into exit code 1. - * ``SIGTERM`` / ``kill -ABRT``: ``close(exit_code=128 + signum)`` — full - teardown before exiting on isaacsim.simulation_app >= 2.18.5, truthful - status either way; if ``close()`` returns (fast shutdown disabled), - re-raise the signal with the default action. + * ``SIGTERM``: ``close(exit_code=128 + signum)``; full teardown before + exiting on isaacsim.simulation_app >= 2.18.5, truthful status either way; + if ``close()`` returns (fast shutdown disabled), re-raise the signal with + the default action. * Signal during a running close: fall back to the default action instead of re-entering ``close()``, which previously recursed until stack overflow. - * ``SIGSEGV``: not intercepted — a Python handler cannot run for main-thread - faults, reports success for worker-thread faults, and suppresses the carb - crash reporter's minidumps. + * ``SIGSEGV`` / ``SIGABRT``: not intercepted; left at ``SIG_DFL`` so fatal + faults terminate and can produce core dumps. a python handler that returns + from ``SIGSEGV`` re-faults forever and suppresses carb crash minidumps. * ``SIGINT``: Python's default handler, so Ctrl-C unwinds user code and exits nonzero. @@ -1579,8 +1579,9 @@ def install_exit_handlers(self): # close on normal exit so Kit shuts down cleanly instead of via __del__ atexit.register(self._close_at_exit) signal.signal(signal.SIGTERM, self._on_abort_signal) - signal.signal(signal.SIGABRT, self._on_abort_signal) - # no SIGSEGV handler and default SIGINT — see the class docstring + # leave SIGSEGV/SIGABRT at SIG_DFL; a python handler that returns from a + # synchronous fault re-executes forever and suppresses core dumps. + # restore default SIGINT so ctrl-c unwinds user code; see the class docstring signal.signal(signal.SIGINT, signal.default_int_handler) def _close_at_exit(self): @@ -1593,7 +1594,7 @@ def _close_at_exit(self): self._close_app(exit_code) def _on_abort_signal(self, signum, frame): - """Handle SIGTERM/SIGABRT: close the app once, then die by the signal.""" + """Handle SIGTERM: close the app once, then die by the signal.""" if self._closing: signal.signal(signum, signal.SIG_DFL) signal.raise_signal(signum) diff --git a/source/isaaclab/test/app/test_simulation_app_lifecycle.py b/source/isaaclab/test/app/test_simulation_app_lifecycle.py index f85a2a6e4145..020735e095b9 100644 --- a/source/isaaclab/test/app/test_simulation_app_lifecycle.py +++ b/source/isaaclab/test/app/test_simulation_app_lifecycle.py @@ -160,3 +160,33 @@ def _close(): # simulates a SimulationApp.close() without the exit_code paramet assert calls == [1] assert "does not accept exit_code" in capfd.readouterr().err + +def test_install_exit_handlers_leaves_fatal_signals_at_default(monkeypatch): + """SIGSEGV and SIGABRT must stay at SIG_DFL; only SIGTERM gets a custom handler. + + Regression for issue #7774: a python SIGSEGV handler that returns re-faults + forever with no core dump. SIGABRT is left at the default disposition for the + same reason; graceful teardown stays on SIGTERM only. + """ + actions = _capture_signal_actions(monkeypatch) + lifecycle = _make_lifecycle(lambda exit_code=0: None) + monkeypatch.setattr( + "isaaclab.app.app_launcher.atexit.register", + lambda callback: actions.append(("atexit", callback)), + ) + + lifecycle.install_exit_handlers() + + registered = { + signum: handler + for action in actions + if action[0] == "set_handler" + for _, signum, handler in (action,) + } + assert signal.SIGTERM in registered + assert registered[signal.SIGTERM] == lifecycle._on_abort_signal + assert signal.SIGSEGV not in registered + assert signal.SIGABRT not in registered + assert registered.get(signal.SIGINT) == signal.default_int_handler + assert any(action[0] == "atexit" for action in actions) + From 459c97fbce8812e527a3b0cbde30be45825ed71b Mon Sep 17 00:00:00 2001 From: Dundy Pasupuleti Date: Sat, 12 Sep 2026 21:26:03 +0000 Subject: [PATCH 2/2] fix: address review nits on AppLauncher fatal-signal comments Signed-off-by: Dundy Pasupuleti --- .../fix-applauncher-fatal-signal-defaults.rst | 4 ++-- source/isaaclab/isaaclab/app/app_launcher.py | 15 ++++++++++----- .../test/app/test_simulation_app_lifecycle.py | 11 +++++++---- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst b/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst index 2caf6e4379f0..3645961f8219 100644 --- a/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst +++ b/source/isaaclab/changelog.d/fix-applauncher-fatal-signal-defaults.rst @@ -2,5 +2,5 @@ Fixed ^^^^^ * Fixed :class:`~isaaclab.app.AppLauncher` registering a Python handler for - ``SIGABRT``. fatal signals stay at ``SIG_DFL`` so crashes terminate and can - produce core dumps; graceful shutdown remains on ``SIGTERM`` only. + ``SIGABRT``. fatal-signal dispositions are left untouched (default or + carb's crash handler); graceful shutdown remains on ``SIGTERM`` only. diff --git a/source/isaaclab/isaaclab/app/app_launcher.py b/source/isaaclab/isaaclab/app/app_launcher.py index 2cf3547c39dc..d1b91c3f9abd 100644 --- a/source/isaaclab/isaaclab/app/app_launcher.py +++ b/source/isaaclab/isaaclab/app/app_launcher.py @@ -1541,9 +1541,12 @@ class _SimulationAppLifecycle: the default action. * Signal during a running close: fall back to the default action instead of re-entering ``close()``, which previously recursed until stack overflow. - * ``SIGSEGV`` / ``SIGABRT``: not intercepted; left at ``SIG_DFL`` so fatal - faults terminate and can produce core dumps. a python handler that returns - from ``SIGSEGV`` re-faults forever and suppresses carb crash minidumps. + * ``SIGSEGV`` / ``SIGABRT``: not intercepted; dispositions are left + untouched (default or carb's crash handler). a python ``SIGSEGV`` + handler that returns re-faults forever and suppresses carb crash + minidumps. the former ``SIGABRT`` handler restored ``SIG_DFL`` and + re-raised, so skipping registration leaves abort to the existing + disposition instead of a graceful-teardown path. * ``SIGINT``: Python's default handler, so Ctrl-C unwinds user code and exits nonzero. @@ -1579,8 +1582,10 @@ def install_exit_handlers(self): # close on normal exit so Kit shuts down cleanly instead of via __del__ atexit.register(self._close_at_exit) signal.signal(signal.SIGTERM, self._on_abort_signal) - # leave SIGSEGV/SIGABRT at SIG_DFL; a python handler that returns from a - # synchronous fault re-executes forever and suppresses core dumps. + # leave SIGSEGV/SIGABRT untouched (default or carb's crash handler); + # skipping signal.signal does not install SIG_DFL. a python SIGSEGV + # handler that returns re-executes the fault forever; the former + # SIGABRT handler restored SIG_DFL and re-raised. # restore default SIGINT so ctrl-c unwinds user code; see the class docstring signal.signal(signal.SIGINT, signal.default_int_handler) diff --git a/source/isaaclab/test/app/test_simulation_app_lifecycle.py b/source/isaaclab/test/app/test_simulation_app_lifecycle.py index 020735e095b9..e1a9b90664f2 100644 --- a/source/isaaclab/test/app/test_simulation_app_lifecycle.py +++ b/source/isaaclab/test/app/test_simulation_app_lifecycle.py @@ -161,12 +161,15 @@ def _close(): # simulates a SimulationApp.close() without the exit_code paramet assert calls == [1] assert "does not accept exit_code" in capfd.readouterr().err + def test_install_exit_handlers_leaves_fatal_signals_at_default(monkeypatch): - """SIGSEGV and SIGABRT must stay at SIG_DFL; only SIGTERM gets a custom handler. + """AppLauncher must not register SIGSEGV or SIGABRT; only SIGTERM gets a custom handler. - Regression for issue #7774: a python SIGSEGV handler that returns re-faults - forever with no core dump. SIGABRT is left at the default disposition for the - same reason; graceful teardown stays on SIGTERM only. + Regression for issue #7774: skipping signal.signal leaves those dispositions + untouched (default or carb's crash handler) rather than establishing SIG_DFL. + a python SIGSEGV handler that returns re-faults forever with no core dump; + the former SIGABRT handler restored SIG_DFL and re-raised. graceful teardown + stays on SIGTERM only. """ actions = _capture_signal_actions(monkeypatch) lifecycle = _make_lifecycle(lambda exit_code=0: None)