Use GUsbSource on Linux to avoid threading issueswip/hughsie/GUsbSource

Having two threads interact is causing difficult to debug deadlocks. Just use
libusb_set_pollfd_notifiers() and use the locking in libusb_handle_events().

5 files changed, 224 insertions, 7 deletions

diff --git a/gusb/gusb-context.c b/gusb/gusb-context.c
index f19fd52..88129b2 100644
--- a/gusb/gusb-context.c
+++ b/gusb/gusb-context.c
@@ -19,6 +19,7 @@
 
 #include "gusb-context-private.h"
 #include "gusb-device-private.h"
+#include "gusb-source-private.h"
 #include "gusb-util.h"
 
 enum { PROP_0, PROP_LIBUSB_CONTEXT, PROP_DEBUG_LEVEL, N_PROPERTIES };
@@ -43,6 +44,8 @@ typedef struct {
 	GThread *thread_event;
 	gboolean done_enumerate;
 	volatile gint thread_event_run;
+	GMutex source_mutex;
+	GUsbSource *source;
 	guint hotplug_poll_id;
 	guint hotplug_poll_interval;
 	int debug_level;
@@ -119,7 +122,7 @@ g_usb_context_dispose(GObject *object)
 	GUsbContextPrivate *priv = GET_PRIVATE(self);
 
 	/* this is safe to call even when priv->hotplug_id is unset */
-	if (g_atomic_int_dec_and_test(&priv->thread_event_run)) {
+	if (priv->thread_event != NULL && g_atomic_int_dec_and_test(&priv->thread_event_run)) {
 		libusb_hotplug_deregister_callback(priv->ctx, priv->hotplug_id);
 		g_thread_join(priv->thread_event);
 	}
@@ -132,6 +135,10 @@ g_usb_context_dispose(GObject *object)
 		g_source_remove(priv->idle_events_id);
 		priv->idle_events_id = 0;
 	}
+	if (priv->source != NULL) {
+		_g_usb_source_destroy(priv->source);
+		priv->source = NULL;
+	}
 
 	g_clear_pointer(&priv->main_ctx, g_main_context_unref);
 	g_clear_pointer(&priv->devices, g_ptr_array_unref);
@@ -767,6 +774,9 @@ g_usb_context_enumerate(GUsbContext *self)
 			      g_ptr_array_index(priv->devices, i));
 	}
 
+	/* setup with the default mainloop if not already done */
+	g_usb_context_get_source(self, NULL);
+
 	/* any queued up hotplug events are queued as idle handlers */
 }
 
@@ -804,6 +814,7 @@ g_usb_context_get_flags(GUsbContext *self)
 	return priv->flags;
 }
 
+#ifdef __FreeBSD__
 static gpointer
 g_usb_context_event_thread_cb(gpointer data)
 {
@@ -819,6 +830,7 @@ g_usb_context_event_thread_cb(gpointer data)
 
 	return NULL;
 }
+#endif
 
 static void
 g_usb_context_init(GUsbContext *self)
@@ -831,6 +843,7 @@ g_usb_context_init(GUsbContext *self)
 	priv->devices_removed = g_ptr_array_new_with_free_func((GDestroyNotify)g_object_unref);
 	priv->dict_usb_ids = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
 	priv->dict_replug = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
+	g_mutex_init(&priv->source_mutex);
 
 	/* to escape the thread into the mainloop */
 	g_rec_mutex_init(&priv->idle_events_mutex);
@@ -859,8 +872,12 @@ g_usb_context_initable_init(GInitable *initable, GCancellable *cancellable, GErr
 
 	priv->main_ctx = g_main_context_ref(g_main_context_default());
 	priv->ctx = ctx;
+
+#ifdef __FreeBSD__
+	/* FreeBSD cannot use libusb_set_pollfd_notifiers(), so use a thread instead */
 	priv->thread_event_run = 1;
 	priv->thread_event = g_thread_new("GUsbEventThread", g_usb_context_event_thread_cb, self);
+#endif
 
 	/* watch for add/remove */
 	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG)) {
@@ -910,7 +927,11 @@ _g_usb_context_get_context(GUsbContext *self)
  * @self: a #GUsbContext
  * @main_ctx: a #GMainContext, or %NULL
  *
- * This function does nothing.
+ * Returns a source for this context. The first call actually creates the source and the result
+ * is returned in all future calls, unless threading is being used.
+ *
+ * If the platform does not support libusb_set_pollfd_notifiers() then a thread is being used,
+ * and this function returns %NULL.
  *
  * Return value: (transfer none): the #GUsbSource.
  *
@@ -919,7 +940,16 @@ _g_usb_context_get_context(GUsbContext *self)
 GUsbSource *
 g_usb_context_get_source(GUsbContext *self, GMainContext *main_ctx)
 {
-	return NULL;
+	GUsbContextPrivate *priv = GET_PRIVATE(self);
+	g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&priv->source_mutex);
+
+	g_assert(locker != NULL);
+
+	if (priv->thread_event != NULL)
+		return NULL;
+	if (priv->source == NULL)
+		priv->source = _g_usb_source_new(main_ctx, self);
+	return priv->source;
 }
 
 /**
diff --git a/gusb/gusb-context.h b/gusb/gusb-context.h
index f633520..2b2b56a 100644
--- a/gusb/gusb-context.h
+++ b/gusb/gusb-context.h
@@ -56,7 +56,6 @@ g_usb_context_set_flags(GUsbContext *self, GUsbContextFlags flags);
 GUsbContextFlags
 g_usb_context_get_flags(GUsbContext *self);
 
-G_DEPRECATED
 GUsbSource *
 g_usb_context_get_source(GUsbContext *self, GMainContext *main_ctx);
 GMainContext *
diff --git a/gusb/gusb-device.c b/gusb/gusb-device.c
index 601e5d6..45823b4 100644
--- a/gusb/gusb-device.c
+++ b/gusb/gusb-device.c
@@ -2164,6 +2164,9 @@ g_usb_device_control_transfer_async(GUsbDevice *self,
 							    req,
 							    NULL);
 	}
+
+	/* setup with the default mainloop */
+	g_usb_context_get_source(priv->context, NULL);
 }
 
 /**
@@ -2340,6 +2343,9 @@ g_usb_device_bulk_transfer_async(GUsbDevice *self,
 							    req,
 							    NULL);
 	}
+
+	/* setup with the default mainloop */
+	g_usb_context_get_source(priv->context, NULL);
 }
 
 /**
@@ -2516,6 +2522,9 @@ g_usb_device_interrupt_transfer_async(GUsbDevice *self,
 							    req,
 							    NULL);
 	}
+
+	/* setup with the default mainloop */
+	g_usb_context_get_source(priv->context, NULL);
 }
 
 /**
diff --git a/gusb/gusb-source-private.h b/gusb/gusb-source-private.h
new file mode 100644
index 0000000..61f5be0
--- /dev/null
+++ b/gusb/gusb-source-private.h
@@ -0,0 +1,20 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*-
+ *
+ * Copyright (C) 2010 Richard Hughes <richard@hughsie.com>
+ * Copyright (C) 2011 Debarshi Ray <debarshir@src.gnome.org>
+ *
+ * SPDX-License-Identifier: LGPL-2.1+
+ */
+
+#pragma once
+
+#include <glib.h>
+
+G_BEGIN_DECLS
+
+GUsbSource *
+_g_usb_source_new(GMainContext *main_ctx, GUsbContext *context);
+void
+_g_usb_source_destroy(GUsbSource *source);
+
+G_END_DECLS
diff --git a/gusb/gusb-source.c b/gusb/gusb-source.c
index becb8c8..a6ed19b 100644
--- a/gusb/gusb-source.c
+++ b/gusb/gusb-source.c
@@ -10,13 +10,25 @@
  * SECTION:gusb-source
  * @short_description: GSource integration for libusb
  *
- * This object used to integrate libusb into the GLib main loop before we used
- * a thread. It's now pretty much unused.
+ * This object can be used to integrate libusb into the GLib main loop.
  */
 
 #include "config.h"
 
+#include <libusb-1.0/libusb.h>
+#include <stdlib.h>
+
+#include "gusb-context-private.h"
+#include "gusb-context.h"
+#include "gusb-source-private.h"
 #include "gusb-source.h"
+#include "gusb-util.h"
+
+/* the <poll.h> header is not available on all platforms */
+#ifndef POLLIN
+#define POLLIN	0x0001
+#define POLLOUT 0x0004
+#endif
 
 /**
  * g_usb_source_error_quark:
@@ -34,6 +46,153 @@ g_usb_source_error_quark(void)
 	return quark;
 }
 
+struct _GUsbSource {
+	GSource source;
+	GSList *pollfds;
+	libusb_context *ctx;
+};
+
+static void
+g_usb_source_pollfd_add(GUsbSource *self, int fd, short events)
+{
+	GPollFD *pollfd = g_new0(GPollFD, 1);
+
+	pollfd->fd = fd;
+	pollfd->events = 0;
+	pollfd->revents = 0;
+	if (events & POLLIN)
+		pollfd->events |= G_IO_IN;
+	if (events & POLLOUT)
+		pollfd->events |= G_IO_OUT;
+
+	self->pollfds = g_slist_prepend(self->pollfds, pollfd);
+	g_source_add_poll((GSource *)self, pollfd);
+}
+
+static void
+g_usb_source_pollfd_added_cb(int fd, short events, void *user_data)
+{
+	GUsbSource *self = user_data;
+	g_usb_source_pollfd_add(self, fd, events);
+}
+
+static void
+g_usb_source_pollfd_removed_cb(int fd, void *user_data)
+{
+	GUsbSource *self = user_data;
+
+	/* find the pollfd in the list */
+	for (GSList *elem = self->pollfds; elem != NULL; elem = elem->next) {
+		GPollFD *pollfd = elem->data;
+		if (pollfd->fd == fd) {
+			g_source_remove_poll((GSource *)self, pollfd);
+			g_free(pollfd);
+			self->pollfds = g_slist_delete_link(self->pollfds, elem);
+			return;
+		}
+	}
+	g_warning("couldn't find fd %d in list", fd);
+}
+
+static gboolean
+g_usb_source_prepare(GSource *source, gint *timeout)
+{
+	GUsbSource *self = (GUsbSource *)source;
+
+	/* before the poll */
+	libusb_lock_events(self->ctx);
+
+	/* release the lock at least once per minute */
+	*timeout = 60000;
+	return FALSE;
+}
+
+static gboolean
+g_usb_source_check(GSource *source)
+{
+	GUsbSource *self = (GUsbSource *)source;
+
+	for (GSList *elem = self->pollfds; elem != NULL; elem = elem->next) {
+		GPollFD *pollfd = elem->data;
+		if (pollfd->revents)
+			return TRUE;
+	}
+	libusb_unlock_events(self->ctx);
+	return FALSE;
+}
+
+static gboolean
+g_usb_source_dispatch(GSource *source, GSourceFunc callback, gpointer user_data)
+{
+	GUsbSource *self = (GUsbSource *)source;
+	struct timeval tv = {0, 0};
+	gint rc;
+
+	rc = libusb_handle_events_locked(self->ctx, &tv);
+	libusb_unlock_events(self->ctx);
+	if (rc < 0)
+		g_warning("failed to handle event: %s [%i]", g_usb_strerror(rc), rc);
+
+	if (callback != NULL)
+		callback(user_data);
+
+	return G_SOURCE_CONTINUE;
+}
+
+static void
+g_usb_source_finalize(GSource *source)
+{
+	GUsbSource *self = (GUsbSource *)source;
+	g_slist_free(self->pollfds);
+}
+
+static GSourceFuncs usb_source_funcs = {g_usb_source_prepare,
+					g_usb_source_check,
+					g_usb_source_dispatch,
+					g_usb_source_finalize};
+
+GUsbSource *
+_g_usb_source_new(GMainContext *main_ctx, GUsbContext *gusb_ctx)
+{
+	GUsbSource *self;
+	const struct libusb_pollfd **pollfds;
+
+	self = (GUsbSource *)g_source_new(&usb_source_funcs, sizeof(GUsbSource));
+	self->pollfds = NULL;
+	self->ctx = _g_usb_context_get_context(gusb_ctx);
+
+	/* watch the fd's already created */
+	pollfds = libusb_get_pollfds(self->ctx);
+	for (guint i = 0; pollfds != NULL && pollfds[i] != NULL; i++)
+		g_usb_source_pollfd_add(self, pollfds[i]->fd, pollfds[i]->events);
+	free(pollfds);
+
+	/* watch for PollFD changes */
+	g_source_attach((GSource *)self, main_ctx);
+	libusb_set_pollfd_notifiers(self->ctx,
+				    g_usb_source_pollfd_added_cb,
+				    g_usb_source_pollfd_removed_cb,
+				    self);
+	return self;
+}
+
+static void
+g_usb_source_pollfd_remove_cb(gpointer data, gpointer user_data)
+{
+	GPollFD *pollfd = (GPollFD *)data;
+	GSource *source = (GSource *)user_data;
+	g_source_remove_poll(source, pollfd);
+}
+
+void
+_g_usb_source_destroy(GUsbSource *self)
+{
+	libusb_set_pollfd_notifiers(self->ctx, NULL, NULL, NULL);
+	g_slist_foreach(self->pollfds, g_usb_source_pollfd_remove_cb, self);
+	g_slist_free_full(g_steal_pointer(&self->pollfds), g_free);
+	g_source_destroy((GSource *)self);
+}
+
 /**
  * g_usb_source_set_callback:
  * @self: a #GUsbSource
@@ -41,7 +200,7 @@ g_usb_source_error_quark(void)
  * @data: data to pass to @func
  * @notify: a #GDestroyNotify
  *
- * This function does nothing.
+ * Set a callback to be called when the source is dispatched.
  *
  * Since: 0.1.0
  **/