Skip to content
  • Allison Karlitskaya's avatar
    gsignal: fix closure invalidation code · d89fb7bf
    Allison Karlitskaya authored
    This is the bug that has been causing segfaults and criticals when accel
    keys are used to close windows via GtkUIManager.
    
    The main cause of this problem was a mistake made in the original patch
    when modifying the handler_lookup() to take the extra 'closure'
    parameter.  The original check used was:
    
        if (handler->sequential_number == handler_id ||
           (closure && handler->closure == closure))
    
    It was called to find a particular closure like so:
    
        handler_lookup (instance, 0, closure, &signal_id);
    
    The problem is that the check will return if either the signal ID or
    closure matches (if a closure was given).  The calling code assumes 0 to
    be an invalid signal ID which will match no handlers, but unfortunately
    the rest of gsignal code uses this to denote a signal that has already
    been disconnected.  The result is that this function was searching for a
    matching closure _or_ the first already-disconnected handler.  When it
    found the already-disconnected handler, we'd get criticals and crashes.
    
    The condition has been corrected; it now ignores the handler_id
    parameter if the closure parameter is non-NULL.
    
    While we're in here, change the lifecycle of the invalidation notify to
    be easier to understand.
    
    Before, the notify was removed when the last reference on the handler
    dropped.  This could happen in very many situations; often at the end of
    an emission.  Instead, we now tie the registration of the notifier to
    the lifecycle of the signal connection.  When the signal is disconnected
    we remove the notification, even if other references are held (eg:
    because it is currently being dispatched).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=690118
    d89fb7bf