Gstreamer/PyGObject breaks SIGINT handling

[albertz]

GLib.MainLoop.__init__ installs a SIGINT handler which calls GLib.MainLoop.quit(),
and then reraise a KeyboardInterrupt in that thread.
However, the main thread will not notice that, and continues whatever it is doing, which mostly means that it will hang as it is waiting for something.
What you would want is a KeyboardInterrupt in the main thread, as this is the original behavior when you press Ctrl+C.

I came up with this workaround / ugly monkey patch to just disable the SIGINT handler.
However, a better fix would be in PyGObject to raise the KeyboardInterrupt in the main thread.

def monkeyfix_glib():
  """
  Fixes some stupid bugs such that SIGINT is not working.
  This is used by audioread, and indirectly by librosa for loading audio.
  https://stackoverflow.com/questions/16410852/
  """
  try:
    import gi
  except ImportError:
    return
  try:
    from gi.repository import GLib
  except ImportError:
    from gi.overrides import GLib
  # Do nothing.
  # The original behavior would install a SIGINT handler which calls GLib.MainLoop.quit(),
  # and then reraise a KeyboardInterrupt in that thread.
  # However, we want and expect to get the KeyboardInterrupt in the main thread.
  GLib.MainLoop.__init__ = lambda *args, **kwargs: None

[sampsyo]

Hmm; I don't think I quite understand. What do you mean by "the original behavior" here? Are you saying that GLib itself (or perhaps GStreamer, or perhaps PyGObject—not sure which) installs a SIGINT handler when it's loaded? If the latter, perhaps this bug is best addressed in one of those projects instead of audioread itself?

[albertz]

Sorry, bad formulation. I edited it. PyGObject installs a SIGINT handler, and the main Python thread will never get a KeyboardInterrupt, which is not what you want. You can see that in the code. Actually, not in the upstream code on GitHub (here), but in the Ubuntu 16 version:

class MainLoop(GLib.MainLoop):
    # Backwards compatible constructor API
    def __new__(cls, context=None):
        return GLib.MainLoop.new(context, False)

    # Retain classic pygobject behaviour of quitting main loops on SIGINT
    def __init__(self, context=None):
        def _handler(loop):
            loop.quit()
            loop._quit_by_sigint = True
            # We handle signal deletion in __del__, return True so GLib
            # doesn't do the deletion for us.
            return True

        if sys.platform != 'win32':
            # compatibility shim, keep around until we depend on glib 2.36
            if hasattr(GLib, 'unix_signal_add'):
                fn = GLib.unix_signal_add
            else:
                fn = GLib.unix_signal_add_full
            self._signal_source = fn(GLib.PRIORITY_DEFAULT, signal.SIGINT, _handler, self)

    def __del__(self):
        if hasattr(self, '_signal_source'):
            GLib.source_remove(self._signal_source)

    def run(self):
        super(MainLoop, self).run()
        if hasattr(self, '_quit_by_sigint'):
            # caught by _main_loop_sigint_handler()
            raise KeyboardInterrupt

pip installing a new version also is somehow broken on Ubuntu. I didn't investigated this further.

[sampsyo]

Ah, I see! So perhaps the right thing to do would be to add a KeyboardInterrupt handler around the self.loop.run() call in MainLoopThread and explicitly signal the main thread to stop? That would be a little hairy…

Perhaps this SO answer points to a reasonable alternative?
https://stackoverflow.com/a/16486080/39182

[albertz]

Yes, that would be another solution. You can use thread.interrupt_main() to do that.

The SO solution is not so nice as it would just kill your Python program (I think) and not do the proper handling which you might want for KeyboardInterrupt. Actually, calling audioread should just not change the behavior of that in any way. E.g. consider that the user had installed an own custom handler, which would just get overwritten by this.

[lazka]

fyi, pygobject 3.28+ will no longer install a signal handler there if one was already set before and will no longer change anything when not run in the main thread.

There is an easier fix to work around all this:

instead of GLib.MainLoop() do GLib.MainLoop.new(None, False) in audioread

[sampsyo]

Awesome, @lazka; thank you for the background.

Do you happen to know where in the docs I can learn about the difference between the MainLoop() constructor and the MainLoop.new(None, False) call? These docs, for example, mention the new classmethod, which clearly corresponds to the underlying g_main_loop_new function, but it's not clear what using the constructor was doing.

[lazka]

Do you happen to know where in the docs I can learn about the difference between the MainLoop() constructor and the MainLoop.new(None, False) call? These docs, for example, mention the new classmethod, which clearly corresponds to the underlying g_main_loop_new function, but it's not clear what using the constructor was doing.

Sadly nowhere. __init__ was the (buggy) code @albertz posted above before, now it does nothing and the improved logic is in run() instead. new() returns an already initialized instance (from C) and skips __init__.

[sampsyo]

Got it; thanks again for the background about how this happened!