Merge branch 'tsan' into 'master'

Fix thread safety issues

See merge request GNOME/glib!690
This commit is contained in:
Philip Withnall 2019-03-05 11:46:33 +00:00
commit aeb7dbad30
7 changed files with 38 additions and 43 deletions

View File

@ -43,7 +43,9 @@ enum {
struct _GCancellablePrivate struct _GCancellablePrivate
{ {
guint cancelled : 1; /* Atomic so that g_cancellable_is_cancelled does not require holding the mutex. */
gboolean cancelled;
/* Access to fields below is protected by cancellable_mutex. */
guint cancelled_running : 1; guint cancelled_running : 1;
guint cancelled_running_waiting : 1; guint cancelled_running_waiting : 1;
@ -269,12 +271,12 @@ g_cancellable_reset (GCancellable *cancellable)
g_cond_wait (&cancellable_cond, &cancellable_mutex); g_cond_wait (&cancellable_cond, &cancellable_mutex);
} }
if (priv->cancelled) if (g_atomic_int_get (&priv->cancelled))
{ {
if (priv->wakeup) if (priv->wakeup)
GLIB_PRIVATE_CALL (g_wakeup_acknowledge) (priv->wakeup); GLIB_PRIVATE_CALL (g_wakeup_acknowledge) (priv->wakeup);
priv->cancelled = FALSE; g_atomic_int_set (&priv->cancelled, FALSE);
} }
g_mutex_unlock (&cancellable_mutex); g_mutex_unlock (&cancellable_mutex);
@ -292,7 +294,7 @@ g_cancellable_reset (GCancellable *cancellable)
gboolean gboolean
g_cancellable_is_cancelled (GCancellable *cancellable) g_cancellable_is_cancelled (GCancellable *cancellable)
{ {
return cancellable != NULL && cancellable->priv->cancelled; return cancellable != NULL && g_atomic_int_get (&cancellable->priv->cancelled);
} }
/** /**
@ -404,7 +406,7 @@ g_cancellable_make_pollfd (GCancellable *cancellable, GPollFD *pollfd)
{ {
cancellable->priv->wakeup = GLIB_PRIVATE_CALL (g_wakeup_new) (); cancellable->priv->wakeup = GLIB_PRIVATE_CALL (g_wakeup_new) ();
if (cancellable->priv->cancelled) if (g_atomic_int_get (&cancellable->priv->cancelled))
GLIB_PRIVATE_CALL (g_wakeup_signal) (cancellable->priv->wakeup); GLIB_PRIVATE_CALL (g_wakeup_signal) (cancellable->priv->wakeup);
} }
@ -440,11 +442,11 @@ g_cancellable_release_fd (GCancellable *cancellable)
return; return;
g_return_if_fail (G_IS_CANCELLABLE (cancellable)); g_return_if_fail (G_IS_CANCELLABLE (cancellable));
g_return_if_fail (cancellable->priv->fd_refcount > 0);
priv = cancellable->priv; priv = cancellable->priv;
g_mutex_lock (&cancellable_mutex); g_mutex_lock (&cancellable_mutex);
g_assert (priv->fd_refcount > 0);
priv->fd_refcount--; priv->fd_refcount--;
if (priv->fd_refcount == 0) if (priv->fd_refcount == 0)
@ -482,21 +484,20 @@ g_cancellable_cancel (GCancellable *cancellable)
{ {
GCancellablePrivate *priv; GCancellablePrivate *priv;
if (cancellable == NULL || if (g_cancellable_is_cancelled (cancellable))
cancellable->priv->cancelled)
return; return;
priv = cancellable->priv; priv = cancellable->priv;
g_mutex_lock (&cancellable_mutex); g_mutex_lock (&cancellable_mutex);
if (priv->cancelled) if (g_atomic_int_get (&priv->cancelled))
{ {
g_mutex_unlock (&cancellable_mutex); g_mutex_unlock (&cancellable_mutex);
return; return;
} }
priv->cancelled = TRUE; g_atomic_int_set (&priv->cancelled, TRUE);
priv->cancelled_running = TRUE; priv->cancelled_running = TRUE;
if (priv->wakeup) if (priv->wakeup)
@ -562,7 +563,7 @@ g_cancellable_connect (GCancellable *cancellable,
g_mutex_lock (&cancellable_mutex); g_mutex_lock (&cancellable_mutex);
if (cancellable->priv->cancelled) if (g_atomic_int_get (&cancellable->priv->cancelled))
{ {
void (*_callback) (GCancellable *cancellable, void (*_callback) (GCancellable *cancellable,
gpointer user_data); gpointer user_data);

View File

@ -788,11 +788,11 @@ g_local_file_monitor_start (GLocalFileMonitor *local_monitor,
#endif #endif
} }
g_source_attach ((GSource *) source, context);
G_LOCAL_FILE_MONITOR_GET_CLASS (local_monitor)->start (local_monitor, G_LOCAL_FILE_MONITOR_GET_CLASS (local_monitor)->start (local_monitor,
source->dirname, source->basename, source->filename, source->dirname, source->basename, source->filename,
source); source);
g_source_attach ((GSource *) source, context);
} }
static void static void

View File

@ -583,10 +583,8 @@ on_name_acquired (GDBusConnection *connection,
gpointer user_data) gpointer user_data)
{ {
GMainLoop *loop = user_data; GMainLoop *loop = user_data;
GThread *thread = g_thread_new ("check-proxies", check_proxies_in_thread, loop);
g_thread_new ("check-proxies", g_thread_unref (thread);
check_proxies_in_thread,
loop);
} }
static void static void

View File

@ -309,7 +309,7 @@ struct _GSourceCallback
struct _GMainLoop struct _GMainLoop
{ {
GMainContext *context; GMainContext *context;
gboolean is_running; gboolean is_running; /* (atomic) */
volatile gint ref_count; volatile gint ref_count;
}; };
@ -4087,16 +4087,14 @@ g_main_loop_run (GMainLoop *loop)
LOCK_CONTEXT (loop->context); LOCK_CONTEXT (loop->context);
g_atomic_int_inc (&loop->ref_count); g_atomic_int_inc (&loop->ref_count);
g_atomic_int_set (&loop->is_running, TRUE);
if (!loop->is_running) while (g_atomic_int_get (&loop->is_running) && !got_ownership)
loop->is_running = TRUE;
while (loop->is_running && !got_ownership)
got_ownership = g_main_context_wait_internal (loop->context, got_ownership = g_main_context_wait_internal (loop->context,
&loop->context->cond, &loop->context->cond,
&loop->context->mutex); &loop->context->mutex);
if (!loop->is_running) if (!g_atomic_int_get (&loop->is_running))
{ {
UNLOCK_CONTEXT (loop->context); UNLOCK_CONTEXT (loop->context);
if (got_ownership) if (got_ownership)
@ -4118,8 +4116,8 @@ g_main_loop_run (GMainLoop *loop)
} }
g_atomic_int_inc (&loop->ref_count); g_atomic_int_inc (&loop->ref_count);
loop->is_running = TRUE; g_atomic_int_set (&loop->is_running, TRUE);
while (loop->is_running) while (g_atomic_int_get (&loop->is_running))
g_main_context_iterate (loop->context, TRUE, TRUE, self); g_main_context_iterate (loop->context, TRUE, TRUE, self);
UNLOCK_CONTEXT (loop->context); UNLOCK_CONTEXT (loop->context);
@ -4146,7 +4144,7 @@ g_main_loop_quit (GMainLoop *loop)
g_return_if_fail (g_atomic_int_get (&loop->ref_count) > 0); g_return_if_fail (g_atomic_int_get (&loop->ref_count) > 0);
LOCK_CONTEXT (loop->context); LOCK_CONTEXT (loop->context);
loop->is_running = FALSE; g_atomic_int_set (&loop->is_running, FALSE);
g_wakeup_signal (loop->context->wakeup); g_wakeup_signal (loop->context->wakeup);
g_cond_broadcast (&loop->context->cond); g_cond_broadcast (&loop->context->cond);
@ -4170,7 +4168,7 @@ g_main_loop_is_running (GMainLoop *loop)
g_return_val_if_fail (loop != NULL, FALSE); g_return_val_if_fail (loop != NULL, FALSE);
g_return_val_if_fail (g_atomic_int_get (&loop->ref_count) > 0, FALSE); g_return_val_if_fail (g_atomic_int_get (&loop->ref_count) > 0, FALSE);
return loop->is_running; return g_atomic_int_get (&loop->is_running);
} }
/** /**

View File

@ -263,7 +263,6 @@ g_object_notify_queue_thaw (GObject *object,
GSList *slist; GSList *slist;
guint n_pspecs = 0; guint n_pspecs = 0;
g_return_if_fail (nqueue->freeze_count > 0);
g_return_if_fail (g_atomic_int_get(&object->ref_count) > 0); g_return_if_fail (g_atomic_int_get(&object->ref_count) > 0);
G_LOCK(notify_lock); G_LOCK(notify_lock);
@ -1101,7 +1100,7 @@ void
g_object_run_dispose (GObject *object) g_object_run_dispose (GObject *object)
{ {
g_return_if_fail (G_IS_OBJECT (object)); g_return_if_fail (G_IS_OBJECT (object));
g_return_if_fail (object->ref_count > 0); g_return_if_fail (g_atomic_int_get (&object->ref_count) > 0);
g_object_ref (object); g_object_ref (object);
TRACE (GOBJECT_OBJECT_DISPOSE(object,G_TYPE_FROM_INSTANCE(object), 0)); TRACE (GOBJECT_OBJECT_DISPOSE(object,G_TYPE_FROM_INSTANCE(object), 0));
@ -2819,7 +2818,7 @@ g_object_weak_ref (GObject *object,
g_return_if_fail (G_IS_OBJECT (object)); g_return_if_fail (G_IS_OBJECT (object));
g_return_if_fail (notify != NULL); g_return_if_fail (notify != NULL);
g_return_if_fail (object->ref_count >= 1); g_return_if_fail (g_atomic_int_get (&object->ref_count) >= 1);
G_LOCK (weak_refs_mutex); G_LOCK (weak_refs_mutex);
wstack = g_datalist_id_remove_no_notify (&object->qdata, quark_weak_refs); wstack = g_datalist_id_remove_no_notify (&object->qdata, quark_weak_refs);
@ -3001,7 +3000,7 @@ gpointer
GObject *object = _object; GObject *object = _object;
gboolean was_floating; gboolean was_floating;
g_return_val_if_fail (G_IS_OBJECT (object), object); g_return_val_if_fail (G_IS_OBJECT (object), object);
g_return_val_if_fail (object->ref_count >= 1, object); g_return_val_if_fail (g_atomic_int_get (&object->ref_count) >= 1, object);
g_object_ref (object); g_object_ref (object);
was_floating = floating_flag_handler (object, -1); was_floating = floating_flag_handler (object, -1);
if (was_floating) if (was_floating)
@ -3024,7 +3023,7 @@ void
g_object_force_floating (GObject *object) g_object_force_floating (GObject *object)
{ {
g_return_if_fail (G_IS_OBJECT (object)); g_return_if_fail (G_IS_OBJECT (object));
g_return_if_fail (object->ref_count >= 1); g_return_if_fail (g_atomic_int_get (&object->ref_count) >= 1);
floating_flag_handler (object, +1); floating_flag_handler (object, +1);
} }
@ -3105,7 +3104,7 @@ g_object_add_toggle_ref (GObject *object,
g_return_if_fail (G_IS_OBJECT (object)); g_return_if_fail (G_IS_OBJECT (object));
g_return_if_fail (notify != NULL); g_return_if_fail (notify != NULL);
g_return_if_fail (object->ref_count >= 1); g_return_if_fail (g_atomic_int_get (&object->ref_count) >= 1);
g_object_ref (object); g_object_ref (object);
@ -4139,7 +4138,7 @@ g_object_watch_closure (GObject *object,
g_return_if_fail (closure != NULL); g_return_if_fail (closure != NULL);
g_return_if_fail (closure->is_invalid == FALSE); g_return_if_fail (closure->is_invalid == FALSE);
g_return_if_fail (closure->in_marshal == FALSE); g_return_if_fail (closure->in_marshal == FALSE);
g_return_if_fail (object->ref_count > 0); /* this doesn't work on finalizing objects */ g_return_if_fail (g_atomic_int_get (&object->ref_count) > 0); /* this doesn't work on finalizing objects */
g_closure_add_invalidate_notifier (closure, object, object_remove_closure); g_closure_add_invalidate_notifier (closure, object, object_remove_closure);
g_closure_add_marshal_guards (closure, g_closure_add_marshal_guards (closure,
@ -4185,7 +4184,7 @@ g_closure_new_object (guint sizeof_closure,
GClosure *closure; GClosure *closure;
g_return_val_if_fail (G_IS_OBJECT (object), NULL); g_return_val_if_fail (G_IS_OBJECT (object), NULL);
g_return_val_if_fail (object->ref_count > 0, NULL); /* this doesn't work on finalizing objects */ g_return_val_if_fail (g_atomic_int_get (&object->ref_count) > 0, NULL); /* this doesn't work on finalizing objects */
closure = g_closure_new_simple (sizeof_closure, object); closure = g_closure_new_simple (sizeof_closure, object);
g_object_watch_closure (object, closure); g_object_watch_closure (object, closure);
@ -4213,7 +4212,7 @@ g_cclosure_new_object (GCallback callback_func,
GClosure *closure; GClosure *closure;
g_return_val_if_fail (G_IS_OBJECT (object), NULL); g_return_val_if_fail (G_IS_OBJECT (object), NULL);
g_return_val_if_fail (object->ref_count > 0, NULL); /* this doesn't work on finalizing objects */ g_return_val_if_fail (g_atomic_int_get (&object->ref_count) > 0, NULL); /* this doesn't work on finalizing objects */
g_return_val_if_fail (callback_func != NULL, NULL); g_return_val_if_fail (callback_func != NULL, NULL);
closure = g_cclosure_new (callback_func, object, NULL); closure = g_cclosure_new (callback_func, object, NULL);
@ -4242,7 +4241,7 @@ g_cclosure_new_object_swap (GCallback callback_func,
GClosure *closure; GClosure *closure;
g_return_val_if_fail (G_IS_OBJECT (object), NULL); g_return_val_if_fail (G_IS_OBJECT (object), NULL);
g_return_val_if_fail (object->ref_count > 0, NULL); /* this doesn't work on finalizing objects */ g_return_val_if_fail (g_atomic_int_get (&object->ref_count) > 0, NULL); /* this doesn't work on finalizing objects */
g_return_val_if_fail (callback_func != NULL, NULL); g_return_val_if_fail (callback_func != NULL, NULL);
closure = g_cclosure_new_swap (callback_func, object, NULL); closure = g_cclosure_new_swap (callback_func, object, NULL);

View File

@ -1561,8 +1561,7 @@ g_param_spec_get_default_value (GParamSpec *pspec)
g_param_value_set_default (pspec, &default_value); g_param_value_set_default (pspec, &default_value);
/* store all but the type */ /* store all but the type */
default_value.g_type = 0; memcpy (priv->default_value.data, default_value.data, sizeof (default_value.data));
priv->default_value = default_value;
g_once_init_leave (&priv->default_value.g_type, pspec->value_type); g_once_init_leave (&priv->default_value.g_type, pspec->value_type);
} }

View File

@ -35,7 +35,7 @@ struct _GTestClass
}; };
static GType my_test_get_type (void); static GType my_test_get_type (void);
static volatile gboolean stopping; static gboolean stopping;
static void my_test_class_init (GTestClass * klass); static void my_test_class_init (GTestClass * klass);
static void my_test_init (GTest * test); static void my_test_init (GTest * test);
@ -177,7 +177,7 @@ run_thread (GTest * test)
{ {
gint i = 1; gint i = 1;
while (!stopping) { while (!g_atomic_int_get (&stopping)) {
my_test_do_property (test); my_test_do_property (test);
if ((i++ % 10000) == 0) if ((i++ % 10000) == 0)
{ {
@ -210,14 +210,14 @@ main (int argc, char **argv)
g_signal_connect (test, "notify::dummy", G_CALLBACK (dummy_notify), NULL); g_signal_connect (test, "notify::dummy", G_CALLBACK (dummy_notify), NULL);
} }
stopping = FALSE; g_atomic_int_set (&stopping, FALSE);
for (i = 0; i < N_THREADS; i++) for (i = 0; i < N_THREADS; i++)
test_threads[i] = g_thread_create ((GThreadFunc) run_thread, test_objects[i], TRUE, NULL); test_threads[i] = g_thread_create ((GThreadFunc) run_thread, test_objects[i], TRUE, NULL);
g_usleep (3000000); g_usleep (3000000);
stopping = TRUE; g_atomic_int_set (&stopping, TRUE);
g_print ("\nstopping\n"); g_print ("\nstopping\n");
/* join all threads */ /* join all threads */