luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-01-14 00:06:24 +01:00
Code Issues Packages Projects Releases Wiki Activity

gsignal: Perform signal unlocked handlers block, unblock and disconnect ops

Browse Source 
We used to perform unneeded lock/unlock dances to perform block, unblock
and disconnect actions, and these were potentially unsafe because we
might have looped in data that could be potentially be changed by other
threads.

We could have also done the same by saving the handlers ids in a
temporary array and eventually remove them, but I don't see a reason for
that since we can just keep all locked without the risk of creating
deadlocks.

Coverity CID: #1474757, #1474771, #1474429
This commit is contained in:
Marco Trevisan (Treviño) 2022-07-15 16:05:23 +02:00
parent c1eb93d2a2
commit ae14f3219a
1 changed files with 68 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
gobject/gsignal.c
View File

@ -2618,6 +2618,10 @@ g_signal_connect_data (gpointer instance,
return handler_seq_no; return handler_seq_no;
} }
static void
signal_handler_block_unlocked (gpointer instance,
gulong handler_id);
/** /**
* g_signal_handler_block: * g_signal_handler_block:
* @instance: (type GObject.Object): The instance to block the signal handler of. * @instance: (type GObject.Object): The instance to block the signal handler of.
@ -2636,12 +2640,20 @@ void
g_signal_handler_block (gpointer instance, g_signal_handler_block (gpointer instance,
gulong handler_id) gulong handler_id)
{ {
Handler *handler;
g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance)); g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
g_return_if_fail (handler_id > 0); g_return_if_fail (handler_id > 0);
SIGNAL_LOCK (); SIGNAL_LOCK ();
signal_handler_block_unlocked (instance, handler_id);
SIGNAL_UNLOCK ();
}
static void
signal_handler_block_unlocked (gpointer instance,
gulong handler_id)
{
Handler *handler;
handler = handler_lookup (instance, handler_id, NULL, NULL); handler = handler_lookup (instance, handler_id, NULL, NULL);
if (handler) if (handler)
{ {
@ -2653,9 +2665,12 @@ g_signal_handler_block (gpointer instance,
} }
else else
g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id); g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
SIGNAL_UNLOCK ();
} }
static void
signal_handler_unblock_unlocked (gpointer instance,
gulong handler_id);
/** /**
* g_signal_handler_unblock: * g_signal_handler_unblock:
* @instance: (type GObject.Object): The instance to unblock the signal handler of. * @instance: (type GObject.Object): The instance to unblock the signal handler of.
@ -2679,12 +2694,20 @@ void
g_signal_handler_unblock (gpointer instance, g_signal_handler_unblock (gpointer instance,
gulong handler_id) gulong handler_id)
{ {
Handler *handler;
g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance)); g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
g_return_if_fail (handler_id > 0); g_return_if_fail (handler_id > 0);
SIGNAL_LOCK (); SIGNAL_LOCK ();
signal_handler_unblock_unlocked (instance, handler_id);
SIGNAL_UNLOCK ();
}
static void
signal_handler_unblock_unlocked (gpointer instance,
gulong handler_id)
{
Handler *handler;
handler = handler_lookup (instance, handler_id, NULL, NULL); handler = handler_lookup (instance, handler_id, NULL, NULL);
if (handler) if (handler)
{ {
@ -2695,9 +2718,12 @@ g_signal_handler_unblock (gpointer instance,
} }
else else
g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id); g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
SIGNAL_UNLOCK ();
} }
static void
signal_handler_disconnect_unlocked (gpointer instance,
gulong handler_id);
/** /**
* g_signal_handler_disconnect: * g_signal_handler_disconnect:
* @instance: (type GObject.Object): The instance to remove the signal handler from. * @instance: (type GObject.Object): The instance to remove the signal handler from.
@ -2714,12 +2740,20 @@ void
g_signal_handler_disconnect (gpointer instance, g_signal_handler_disconnect (gpointer instance,
gulong handler_id) gulong handler_id)
{ {
Handler *handler;
g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance)); g_return_if_fail (G_TYPE_CHECK_INSTANCE (instance));
g_return_if_fail (handler_id > 0); g_return_if_fail (handler_id > 0);
SIGNAL_LOCK (); SIGNAL_LOCK ();
signal_handler_disconnect_unlocked (instance, handler_id);
SIGNAL_UNLOCK ();
}
static void
signal_handler_disconnect_unlocked (gpointer instance,
gulong handler_id)
{
Handler *handler;
handler = handler_lookup (instance, handler_id, 0, 0); handler = handler_lookup (instance, handler_id, 0, 0);
if (handler) if (handler)
{ {
@ -2731,7 +2765,6 @@ g_signal_handler_disconnect (gpointer instance,
} }
else else
g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id); g_warning ("%s: instance '%p' has no handler with id '%lu'", G_STRLOC, instance, handler_id);
SIGNAL_UNLOCK ();
} }
/** /**
@ -2862,33 +2895,31 @@ g_signal_handler_find (gpointer instance,
return handler_seq_no; return handler_seq_no;
} }
typedef void (*CallbackHandlerFunc) (gpointer instance, gulong handler_seq_no);
static guint static guint
signal_handlers_foreach_matched_R (gpointer instance, signal_handlers_foreach_matched_unlocked_R (gpointer instance,
GSignalMatchType mask, GSignalMatchType mask,
guint signal_id, guint signal_id,
GQuark detail, GQuark detail,
GClosure *closure, GClosure *closure,
gpointer func, gpointer func,
gpointer data, gpointer data,
void (*callback) (gpointer instance, CallbackHandlerFunc callback)
gulong handler_seq_no))
{ {
HandlerMatch *mlist; HandlerMatch *mlist;
guint n_handlers = 0; guint n_handlers = 0;
mlist = handlers_find (instance, mask, signal_id, detail, closure, func, data, FALSE); mlist = handlers_find (instance, mask, signal_id, detail, closure, func, data, FALSE);
while (mlist) while (mlist)
{ {
n_handlers++; n_handlers++;
if (mlist->handler->sequential_number) if (mlist->handler->sequential_number)
{ callback (instance, mlist->handler->sequential_number);
SIGNAL_UNLOCK ();
callback (instance, mlist->handler->sequential_number);
SIGNAL_LOCK ();
}
mlist = handler_match_free1_R (mlist, instance); mlist = handler_match_free1_R (mlist, instance);
} }
return n_handlers; return n_handlers;
} }
@ -2930,9 +2961,10 @@ g_signal_handlers_block_matched (gpointer instance,
if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA)) if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
{ {
SIGNAL_LOCK (); SIGNAL_LOCK ();
n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail, n_handlers =
closure, func, data, signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
g_signal_handler_block); closure, func, data,
signal_handler_block_unlocked);
SIGNAL_UNLOCK (); SIGNAL_UNLOCK ();
} }
@ -2978,9 +3010,10 @@ g_signal_handlers_unblock_matched (gpointer instance,
if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA)) if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
{ {
SIGNAL_LOCK (); SIGNAL_LOCK ();
n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail, n_handlers =
closure, func, data, signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
g_signal_handler_unblock); closure, func, data,
signal_handler_unblock_unlocked);
SIGNAL_UNLOCK (); SIGNAL_UNLOCK ();
} }
@ -3026,9 +3059,10 @@ g_signal_handlers_disconnect_matched (gpointer instance,
if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA)) if (mask & (G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA))
{ {
SIGNAL_LOCK (); SIGNAL_LOCK ();
n_handlers = signal_handlers_foreach_matched_R (instance, mask, signal_id, detail, n_handlers =
closure, func, data, signal_handlers_foreach_matched_unlocked_R (instance, mask, signal_id, detail,
g_signal_handler_disconnect); closure, func, data,
signal_handler_disconnect_unlocked);
SIGNAL_UNLOCK (); SIGNAL_UNLOCK ();
} }