From 26d5d23f970a4dce99a97286c7f9c8d68d8f5797 Mon Sep 17 00:00:00 2001 From: Robin Jadoul Date: Thu, 2 May 2024 14:17:08 +0200 Subject: [PATCH] Avoid signal handling reentrancy issues If e.g. two SIGUSR1 signals arrive at almost the same time, it can be that the first one hasn't finished refreshing the UI yet when the second one needs to be handled, leading to a generator being executed twice, which python doesn't like. We try to avoid this by scheduling the actual handling logic as a coroutine on the currently running event loop. This has the advantage that the signal handler now has full access to any async functions or synchronization primitives. As long as the event loop is single-threaded, the current version should avoid this particular crash as the refreshing logic is not async and hence cannot be interrupted at the event loop level. If this were to change at some point, an asyncio.Semaphore or equivalent primitive can be easily introduced. --- alot/ui.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/alot/ui.py b/alot/ui.py index 2c47877fe..edc01010e 100644 --- a/alot/ui.py +++ b/alot/ui.py @@ -95,8 +95,8 @@ def __init__(self, dbman, initialcmdline): mainframe = urwid.Frame(urwid.SolidFill()) self.root_widget = urwid.AttrMap(mainframe, global_att) - signal.signal(signal.SIGINT, self.handle_signal) - signal.signal(signal.SIGUSR1, self.handle_signal) + signal.signal(signal.SIGINT, self._handle_signal) + signal.signal(signal.SIGUSR1, self._handle_signal) # load histories self._cache = os.path.join( @@ -728,7 +728,16 @@ async def apply_command(self, cmd): logging.info('calling post-hook') await cmd.posthook(ui=self, dbm=self.dbman, cmd=cmd) - def handle_signal(self, signum, frame): + def _handle_signal(self, signum, _frame): + """ + Handle UNIX signals: add a new task onto the event loop. + Doing it this way ensures what our handler has access to whatever + synchronization primitives or async calls it may require. + """ + loop = asyncio.get_event_loop() + asyncio.run_coroutine_threadsafe(self.handle_signal(signum), loop) + + async def handle_signal(self, signum): """ handles UNIX signals @@ -736,13 +745,11 @@ def handle_signal(self, signum, frame): handle more :param signum: The signal number (see man 7 signal) - :param frame: The execution frame - (https://docs.python.org/2/reference/datamodel.html#frame-objects) """ # it is a SIGINT ? if signum == signal.SIGINT: logging.info('shut down cleanly') - asyncio.ensure_future(self.apply_command(globals.ExitCommand())) + await self.apply_command(globals.ExitCommand()) elif signum == signal.SIGUSR1: if isinstance(self.current_buffer, SearchBuffer): self.current_buffer.rebuild()