gdbus: Avoid blocking on worker thread in connection initialization

I can't see a reason to spin until the worker thread runs, so don't.
This avoids ugly sched_yield() calls that show up in strace and
annoy me; the code is cleaner now too.

We now grab the types needed for the WebKit workaround in the
thread creation area, but only release them when the thread itself
exits.

https://bugzilla.gnome.org/show_bug.cgi?id=651650
This commit is contained in:
Colin Walters 2011-06-01 15:11:02 -04:00
parent a588974561
commit 7ed328aaf0

View File

@ -55,6 +55,8 @@
#include "glibintl.h" #include "glibintl.h"
static gboolean _g_dbus_worker_do_initial_read (gpointer data);
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
gchar * gchar *
@ -240,7 +242,7 @@ ensure_type (GType gtype)
} }
static void static void
released_required_types (void) release_required_types (void)
{ {
g_ptr_array_foreach (ensured_classes, (GFunc) g_type_class_unref, NULL); g_ptr_array_foreach (ensured_classes, (GFunc) g_type_class_unref, NULL);
g_ptr_array_unref (ensured_classes); g_ptr_array_unref (ensured_classes);
@ -257,131 +259,75 @@ ensure_required_types (void)
} }
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
G_LOCK_DEFINE_STATIC (shared_thread_lock);
typedef struct typedef struct
{ {
gint num_users; volatile gint refcount;
GThread *thread; GThread *thread;
GMainContext *context; GMainContext *context;
GMainLoop *loop; GMainLoop *loop;
} SharedThreadData; } SharedThreadData;
static SharedThreadData *shared_thread_data = NULL;
static gpointer static gpointer
gdbus_shared_thread_func (gpointer data) gdbus_shared_thread_func (gpointer user_data)
{ {
g_main_context_push_thread_default (shared_thread_data->context); SharedThreadData *data = user_data;
g_main_loop_run (shared_thread_data->loop);
g_main_context_pop_thread_default (shared_thread_data->context); g_main_context_push_thread_default (data->context);
g_main_loop_run (data->loop);
g_main_context_pop_thread_default (data->context);
release_required_types ();
return NULL; return NULL;
} }
typedef void (*GDBusSharedThreadFunc) (gpointer user_data);
typedef struct
{
GDBusSharedThreadFunc func;
gpointer user_data;
gboolean done;
} CallerData;
static gboolean
invoke_caller (gpointer user_data)
{
CallerData *data = user_data;
data->func (data->user_data);
data->done = TRUE;
return FALSE;
}
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
static void static SharedThreadData *
_g_dbus_shared_thread_ref (GDBusSharedThreadFunc func, _g_dbus_shared_thread_ref (void)
gpointer user_data)
{ {
GError *error; static gsize shared_thread_data = 0;
GSource *idle_source; GError *error = NULL;
CallerData *data; SharedThreadData *ret;
gboolean release_types;
G_LOCK (shared_thread_lock); if (g_once_init_enter (&shared_thread_data))
release_types = FALSE;
if (shared_thread_data != NULL)
{ {
shared_thread_data->num_users += 1; SharedThreadData *data;
goto have_thread;
}
shared_thread_data = g_new0 (SharedThreadData, 1);
shared_thread_data->num_users = 1;
/* Work-around for https://bugzilla.gnome.org/show_bug.cgi?id=627724 */ /* Work-around for https://bugzilla.gnome.org/show_bug.cgi?id=627724 */
ensure_required_types (); ensure_required_types ();
release_types = TRUE;
error = NULL; data = g_new0 (SharedThreadData, 1);
shared_thread_data->context = g_main_context_new (); data->refcount = 0;
shared_thread_data->loop = g_main_loop_new (shared_thread_data->context, FALSE);
shared_thread_data->thread = g_thread_create (gdbus_shared_thread_func, data->context = g_main_context_new ();
NULL, data->loop = g_main_loop_new (data->context, FALSE);
data->thread = g_thread_create (gdbus_shared_thread_func,
data,
TRUE, TRUE,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
/* We can cast between gsize and gpointer safely */
g_once_init_leave (&shared_thread_data, (gsize) data);
}
have_thread: ret = (SharedThreadData*) shared_thread_data;
g_atomic_int_inc (&ret->refcount);
data = g_new0 (CallerData, 1); return ret;
data->func = func;
data->user_data = user_data;
data->done = FALSE;
idle_source = g_idle_source_new ();
g_source_set_priority (idle_source, G_PRIORITY_DEFAULT);
g_source_set_callback (idle_source,
invoke_caller,
data,
NULL);
g_source_attach (idle_source, shared_thread_data->context);
g_source_unref (idle_source);
/* wait for the user code to run.. hmm.. probably use a condition variable instead */
while (!data->done)
g_thread_yield ();
if (release_types)
released_required_types ();
g_free (data);
G_UNLOCK (shared_thread_lock);
} }
static void static void
_g_dbus_shared_thread_unref (void) _g_dbus_shared_thread_unref (SharedThreadData *data)
{ {
/* TODO: actually destroy the shared thread here */ /* TODO: actually destroy the shared thread here */
#if 0 #if 0
G_LOCK (shared_thread_lock); g_assert (data != NULL);
g_assert (shared_thread_data != NULL); if (g_atomic_int_dec_and_test (&data->refcount))
shared_thread_data->num_users -= 1;
if (shared_thread_data->num_users == 0)
{ {
g_main_loop_quit (shared_thread_data->loop); g_main_loop_quit (data->loop);
//g_thread_join (shared_thread_data->thread); //g_thread_join (data->thread);
g_main_loop_unref (shared_thread_data->loop); g_main_loop_unref (data->loop);
g_main_context_unref (shared_thread_data->context); g_main_context_unref (data->context);
g_free (shared_thread_data);
shared_thread_data = NULL;
G_UNLOCK (shared_thread_lock);
}
else
{
G_UNLOCK (shared_thread_lock);
} }
#endif #endif
} }
@ -392,6 +338,8 @@ struct GDBusWorker
{ {
volatile gint ref_count; volatile gint ref_count;
SharedThreadData *shared_thread_data;
gboolean stopped; gboolean stopped;
/* TODO: frozen (e.g. G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING) currently /* TODO: frozen (e.g. G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING) currently
@ -409,8 +357,6 @@ struct GDBusWorker
GDBusWorkerDisconnectedCallback disconnected_callback; GDBusWorkerDisconnectedCallback disconnected_callback;
gpointer user_data; gpointer user_data;
GThread *thread;
/* if not NULL, stream is GSocketConnection */ /* if not NULL, stream is GSocketConnection */
GSocket *socket; GSocket *socket;
@ -470,7 +416,7 @@ _g_dbus_worker_unref (GDBusWorker *worker)
{ {
g_assert (worker->write_pending_flushes == NULL); g_assert (worker->write_pending_flushes == NULL);
_g_dbus_shared_thread_unref (); _g_dbus_shared_thread_unref (worker->shared_thread_data);
g_object_unref (worker->stream); g_object_unref (worker->stream);
@ -575,7 +521,7 @@ _g_dbus_worker_unfreeze (GDBusWorker *worker)
unfreeze_in_idle_cb, unfreeze_in_idle_cb,
_g_dbus_worker_ref (worker), _g_dbus_worker_ref (worker),
(GDestroyNotify) _g_dbus_worker_unref); (GDestroyNotify) _g_dbus_worker_unref);
g_source_attach (idle_source, shared_thread_data->context); g_source_attach (idle_source, worker->shared_thread_data->context);
g_source_unref (idle_source); g_source_unref (idle_source);
} }
@ -850,12 +796,14 @@ _g_dbus_worker_do_read_unlocked (GDBusWorker *worker)
} }
/* called in private thread shared by all GDBusConnection instances (without read-lock held) */ /* called in private thread shared by all GDBusConnection instances (without read-lock held) */
static void static gboolean
_g_dbus_worker_do_read (GDBusWorker *worker) _g_dbus_worker_do_initial_read (gpointer data)
{ {
GDBusWorker *worker = data;
g_mutex_lock (worker->read_lock); g_mutex_lock (worker->read_lock);
_g_dbus_worker_do_read_unlocked (worker); _g_dbus_worker_do_read_unlocked (worker);
g_mutex_unlock (worker->read_lock); g_mutex_unlock (worker->read_lock);
return FALSE;
} }
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
@ -1389,7 +1337,7 @@ _g_dbus_worker_send_message (GDBusWorker *worker,
write_message_in_idle_cb, write_message_in_idle_cb,
_g_dbus_worker_ref (worker), _g_dbus_worker_ref (worker),
(GDestroyNotify) _g_dbus_worker_unref); (GDestroyNotify) _g_dbus_worker_unref);
g_source_attach (idle_source, shared_thread_data->context); g_source_attach (idle_source, worker->shared_thread_data->context);
g_source_unref (idle_source); g_source_unref (idle_source);
} }
g_mutex_unlock (worker->write_lock); g_mutex_unlock (worker->write_lock);
@ -1397,17 +1345,6 @@ _g_dbus_worker_send_message (GDBusWorker *worker,
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
static void
_g_dbus_worker_thread_begin_func (gpointer user_data)
{
GDBusWorker *worker = user_data;
worker->thread = g_thread_self ();
/* begin reading */
_g_dbus_worker_do_read (worker);
}
GDBusWorker * GDBusWorker *
_g_dbus_worker_new (GIOStream *stream, _g_dbus_worker_new (GIOStream *stream,
GDBusCapabilityFlags capabilities, GDBusCapabilityFlags capabilities,
@ -1418,6 +1355,7 @@ _g_dbus_worker_new (GIOStream *stream,
gpointer user_data) gpointer user_data)
{ {
GDBusWorker *worker; GDBusWorker *worker;
GSource *idle_source;
g_return_val_if_fail (G_IS_IO_STREAM (stream), NULL); g_return_val_if_fail (G_IS_IO_STREAM (stream), NULL);
g_return_val_if_fail (message_received_callback != NULL, NULL); g_return_val_if_fail (message_received_callback != NULL, NULL);
@ -1446,7 +1384,17 @@ _g_dbus_worker_new (GIOStream *stream,
if (G_IS_SOCKET_CONNECTION (worker->stream)) if (G_IS_SOCKET_CONNECTION (worker->stream))
worker->socket = g_socket_connection_get_socket (G_SOCKET_CONNECTION (worker->stream)); worker->socket = g_socket_connection_get_socket (G_SOCKET_CONNECTION (worker->stream));
_g_dbus_shared_thread_ref (_g_dbus_worker_thread_begin_func, worker); worker->shared_thread_data = _g_dbus_shared_thread_ref ();
/* begin reading */
idle_source = g_idle_source_new ();
g_source_set_priority (idle_source, G_PRIORITY_DEFAULT);
g_source_set_callback (idle_source,
_g_dbus_worker_do_initial_read,
worker,
NULL);
g_source_attach (idle_source, worker->shared_thread_data->context);
g_source_unref (idle_source);
return worker; return worker;
} }