GNOME Bugzilla – Bug 695683
Regression in KeyboardInterrupt handling
Last modified: 2013-09-11 10:02:23 UTC
Created attachment 238670 [details] The test case The test case: - Run the attached script - Press Ctrl+C twice With PyGObject 3.6, that will exit the script (and that's what one may expect). With PyGObject 3.8, this doesn't exit the script.
Confirmed. At first I thought this might be a dup of bug 622084 but I was wrong. Seems like it might be fallout from https://git.gnome.org/browse/pygobject/commit/?id=191cf45a
This seems to be caused by GLib killing Pythons default signal handler when using GLib.unix_signal_add_full or GLib.unix_signal_source_new In [1]: import signal In [2]: from gi.repository import GLib In [2]: KeyboardInterrupt After creating a unix_signal_source Ctrl+C no longer works In [3]: source = GLib.unix_signal_source_new(signal.SIGINT) In [4]: source.destroy() Restore Pythons default SIGINT handler and Ctrl+C works again In [5]: signal.signal(signal.SIGINT, signal.default_int_handler) Out[5]: <function signal.default_int_handler> In [6]: KeyboardInterrupt So it seems only one SIGINT handler is allowable per process? If this is the case we should be saving the current python SIGINT before a call to run and restoring it after. Likewise we might not want to be creating the signal source in MainLoop.__init__ but right before the call to the base class "run()".
I know little about GTK and the way Python handles signals, but yes, only one signal handler can be installed per process per signal on the OS level.
Created attachment 243439 [details] [review] Add InterruptibleLoopContext for managing SIGINT Add InterruptibleLoopContext context manager for wrapping main loop calls with GLib SIGINT handling for Python.
Review of attachment 243439 [details] [review]: This is a work in progress for an approach I was attempting. The issue ends something in glib is not being cleaned up after adding and remove a signal handler with unix_signal_source_new. This makes it impossible for Pythons signal.signal to work again after the main loop exits. The advantage of InterruptibleLoopContext is that it should be usable for other loop calls (Gtk/Gio.Application.run and Gtk.main). A few broken unitests were added with "skip" showing the problem. I've basically thrown the towel in on this one for now. If anyone wants to pick it up feel free.
Additional note: A potentially fix to the problem would be to look at what the original static bindings did that were removed with: https://git.gnome.org/browse/pygobject/commit/?id=191cf45a Basically it looks like main loop construction would add a signal source watch that looked at PyErr_CheckSignals and exited the main loop accordingly. It might be possible to add a small bit of this back to the static bindings as a util, or even use ctypes to do the same bit of work, thus avoiding usage of unix_signal_source_new.
Created attachment 243554 [details] [review] Add InterruptibleLoopContext for managing SIGINT Updated patch which does not attempt to fix the bug but refactors the MainLoop overrides into the InterruptibleLoopContext context manager. This seems to be well tested by test_mainloop.py and I think can be used immediately to fix bugs like bug 622084. (although those loops will still suffer the same problem described here)
Comment on attachment 243554 [details] [review] Add InterruptibleLoopContext for managing SIGINT Moving this patch over to bug 622084 because it is more relevant there.
Verified this was fixed with bug #704699.