Skip to content
Commit d89fb7bf authored by Allison Karlitskaya's avatar Allison Karlitskaya
Browse files

gsignal: fix closure invalidation code

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
parent 578b657f
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment