Optionally use GUsbSource on Linux to avoid threading issueswip/hughsie/GUsbSource-opt-in

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

By default use a thread to avoid breaking other projects like fprintd that
expect to be able to manually process the default GMainContext.

7 files changed, 266 insertions, 13 deletions

diff --git a/gusb/gusb-context.c b/gusb/gusb-context.c
index f19fd52..edfc8f6 100644
--- a/gusb/gusb-context.c
+++ b/gusb/gusb-context.c
@@ -19,9 +19,10 @@
 
 #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 };
+enum { PROP_0, PROP_LIBUSB_CONTEXT, PROP_DEBUG_LEVEL, PROP_FLAGS, N_PROPERTIES };
 
 enum { DEVICE_ADDED_SIGNAL, DEVICE_REMOVED_SIGNAL, LAST_SIGNAL };
 
@@ -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);
@@ -166,6 +173,9 @@ g_usb_context_get_property(GObject *object, guint prop_id, GValue *value, GParam
 	case PROP_DEBUG_LEVEL:
 		g_value_set_int(value, priv->debug_level);
 		break;
+	case PROP_FLAGS:
+		g_value_set_uint64(value, priv->flags);
+		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
 		break;
@@ -187,6 +197,9 @@ g_usb_context_set_property(GObject *object, guint prop_id, const GValue *value,
 		libusb_set_debug(priv->ctx, priv->debug_level);
 #endif
 		break;
+	case PROP_FLAGS:
+		g_usb_context_set_flags(self, g_value_get_uint64(value));
+		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
 		break;
@@ -215,6 +228,12 @@ g_usb_context_class_init(GUsbContextClass *klass)
 	pspecs[PROP_DEBUG_LEVEL] =
 	    g_param_spec_int("debug_level", NULL, NULL, 0, 3, 0, G_PARAM_READWRITE);
 
+	/**
+	 * GUsbContext:flags:
+	 */
+	pspecs[PROP_FLAGS] =
+	    g_param_spec_uint64("flags", NULL, NULL, 0, G_MAXUINT64, 0, G_PARAM_READWRITE);
+
 	g_object_class_install_properties(object_class, N_PROPERTIES, pspecs);
 
 	/**
@@ -767,6 +786,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 */
 }
 
@@ -784,7 +806,10 @@ void
 g_usb_context_set_flags(GUsbContext *self, GUsbContextFlags flags)
 {
 	GUsbContextPrivate *priv = GET_PRIVATE(self);
+	if (priv->flags == flags)
+		return;
 	priv->flags = flags;
+	g_object_notify_by_pspec(G_OBJECT(self), pspecs[PROP_FLAGS]);
 }
 
 /**
@@ -831,6 +856,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 +885,13 @@ g_usb_context_initable_init(GInitable *initable, GCancellable *cancellable, GErr
 
 	priv->main_ctx = g_main_context_ref(g_main_context_default());
 	priv->ctx = ctx;
-	priv->thread_event_run = 1;
-	priv->thread_event = g_thread_new("GUsbEventThread", g_usb_context_event_thread_cb, self);
+
+	/* FreeBSD cannot use libusb_set_pollfd_notifiers(), so use a thread instead */
+	if (priv->flags & G_USB_CONTEXT_FLAGS_USE_HOTPLUG_THREAD) {
+		priv->thread_event_run = 1;
+		priv->thread_event =
+		    g_thread_new("GUsbEventThread", g_usb_context_event_thread_cb, self);
+	}
 
 	/* watch for add/remove */
 	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG)) {
@@ -910,7 +941,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 +954,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;
 }
 
 /**
@@ -1314,5 +1358,23 @@ g_usb_context_wait_for_replug(GUsbContext *self,
 GUsbContext *
 g_usb_context_new(GError **error)
 {
-	return g_initable_new(G_USB_TYPE_CONTEXT, NULL, error, NULL);
+	return g_usb_context_new_full(G_USB_CONTEXT_FLAGS_USE_HOTPLUG_THREAD, NULL, error);
+}
+
+/**
+ * g_usb_context_new_full:
+ * @flags: a #GUsbContextFlags, e.g. %G_USB_CONTEXT_FLAGS_SAVE_EVENTS
+ * @cancellable: a #GCancellable, or %NULL
+ * @error: a #GError, or %NULL
+ *
+ * Creates a new context for accessing USB devices.
+ *
+ * Return value: a new %GUsbContext object or %NULL on error.
+ *
+ * Since: 0.4.2
+ **/
+GUsbContext *
+g_usb_context_new_full(GUsbContextFlags flags, GCancellable *cancellable, GError **error)
+{
+	return g_initable_new(G_USB_TYPE_CONTEXT, cancellable, error, "flags", flags, NULL);
 }
diff --git a/gusb/gusb-context.h b/gusb/gusb-context.h
index f633520..3c0e898 100644
--- a/gusb/gusb-context.h
+++ b/gusb/gusb-context.h
@@ -41,6 +41,7 @@ typedef enum {
 	G_USB_CONTEXT_FLAGS_NONE = 0,
 	G_USB_CONTEXT_FLAGS_AUTO_OPEN_DEVICES = 1 << 0,
 	G_USB_CONTEXT_FLAGS_SAVE_EVENTS = 1 << 1,
+	G_USB_CONTEXT_FLAGS_USE_HOTPLUG_THREAD = 1 << 2,
 	/*< private >*/
 	G_USB_CONTEXT_FLAGS_LAST
 } GUsbContextFlags;
@@ -50,13 +51,14 @@ g_usb_context_error_quark(void);
 
 GUsbContext *
 g_usb_context_new(GError **error);
+GUsbContext *
+g_usb_context_new_full(GUsbContextFlags flags, GCancellable *cancellable, GError **error);
 
 void
 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..14755a4 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,145 @@ 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)
+{
+	*timeout = 0;
+	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;
+	}
+	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_timeout(self->ctx, &tv);
+	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 +192,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
  **/
diff --git a/gusb/libgusb.ver b/gusb/libgusb.ver
index 4a4a731..2e7eccf 100644
--- a/gusb/libgusb.ver
+++ b/gusb/libgusb.ver
@@ -193,3 +193,9 @@ LIBGUSB_0.4.1 {
     g_usb_device_add_tag;
   local: *;
 } LIBGUSB_0.4.0;
+
+LIBGUSB_0.4.2 {
+  global:
+    g_usb_context_new_full;
+  local: *;
+} LIBGUSB_0.4.1;
diff --git a/tools/gusb-main.c b/tools/gusb-main.c
index 8037310..51efd15 100644
--- a/tools/gusb-main.c
+++ b/tools/gusb-main.c
@@ -516,10 +516,13 @@ main(int argc, char *argv[])
 	}
 
 	/* GUsbContext */
-	priv->usb_ctx = g_usb_context_new(NULL);
 	if (save_events)
 		context_flags |= G_USB_CONTEXT_FLAGS_SAVE_EVENTS;
-	g_usb_context_set_flags(priv->usb_ctx, context_flags);
+	priv->usb_ctx = g_usb_context_new_full(context_flags, NULL, &error);
+	if (priv->usb_ctx == NULL) {
+		g_printerr("%s\n", error->message);
+		return 1;
+	}
 
 	/* add commands */
 	priv->cmd_array = g_ptr_array_new_with_free_func((GDestroyNotify)gusb_cmd_item_free);