From 7b920f5c426e6c1574027254d658dfb8a6798d47 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Mon, 25 Sep 2017 23:50:55 +0200 Subject: [PATCH 1/2] Initialize the usb library only once Initializing the hidapi library more than once is tricky. The hid_init() function can be called multiple times, but the the hid_exit() function will free the resources unconditionally, regardless of how many times the hid_init() function has been called. The consequence is a premature termination of the library. To avoid this problem, the calls are reference counted. Note that this workaround can't protect against calls made outside the libdivecomputer code! The libusb library doesn't suffer from this problem, because each initialization returns a new context pointer. But for consistency, we now also use a single reference counted libusb context. --- src/usbhid.c | 96 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 30 deletions(-) diff --git a/src/usbhid.c b/src/usbhid.c index 38931b8..0f7ba7b 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -52,7 +52,6 @@ struct dc_usbhid_t { dc_context_t *context; /* Internal state. */ #if defined(USE_LIBUSB) - libusb_context *ctx; libusb_device_handle *handle; int interface; unsigned char endpoint_in; @@ -87,13 +86,65 @@ syserror(int errcode) } #endif +#ifdef USBHID +static size_t g_usbhid_refcount = 0; +#ifdef USE_LIBUSB +static libusb_context *g_usbhid_ctx = NULL; +#endif + +static dc_status_t +dc_usbhid_init (dc_context_t *context) +{ + dc_status_t status = DC_STATUS_SUCCESS; + + if (g_usbhid_refcount == 0) { +#if defined(USE_LIBUSB) + int rc = libusb_init (&g_usbhid_ctx); + if (rc != LIBUSB_SUCCESS) { + ERROR (context, "Failed to initialize usb support (%s).", + libusb_error_name (rc)); + status = syserror (rc); + goto error; + } +#elif defined(USE_HIDAPI) + int rc = hid_init(); + if (rc < 0) { + ERROR (context, "Failed to initialize usb support."); + status = DC_STATUS_IO; + goto error; + } +#endif + } + + g_usbhid_refcount++; + +error: + return status; +} + +static dc_status_t +dc_usbhid_exit (void) +{ + + if (--g_usbhid_refcount == 0) { +#if defined(USE_LIBUSB) + libusb_exit (g_usbhid_ctx); + g_usbhid_ctx = NULL; +#elif defined(USE_HIDAPI) + hid_exit (); +#endif + } + + return DC_STATUS_SUCCESS; +} +#endif + dc_status_t dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsigned int pid) { #ifdef USBHID dc_status_t status = DC_STATUS_SUCCESS; dc_usbhid_t *usbhid = NULL; - int rc = 0; if (out == NULL) return DC_STATUS_INVALIDARGS; @@ -110,21 +161,19 @@ dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsi // Library context. usbhid->context = context; -#if defined(USE_LIBUSB) - struct libusb_device **devices = NULL; - struct libusb_config_descriptor *config = NULL; - - // Initialize the libusb library. - rc = libusb_init (&usbhid->ctx); - if (rc != LIBUSB_SUCCESS) { - ERROR (context, "Failed to initialize usb support (%s).", - libusb_error_name (rc)); - status = syserror (rc); + // Initialize the usb library. + status = dc_usbhid_init (context); + if (status != DC_STATUS_SUCCESS) { goto error_free; } +#if defined(USE_LIBUSB) + struct libusb_device **devices = NULL; + struct libusb_config_descriptor *config = NULL; + int rc = 0; + // Enumerate the USB devices. - ssize_t ndevices = libusb_get_device_list (usbhid->ctx, &devices); + ssize_t ndevices = libusb_get_device_list (g_usbhid_ctx, &devices); if (ndevices < 0) { ERROR (context, "Failed to enumerate the usb devices (%s).", libusb_error_name (ndevices)); @@ -243,21 +292,12 @@ dc_usbhid_open (dc_usbhid_t **out, dc_context_t *context, unsigned int vid, unsi libusb_free_device_list (devices, 1); #elif defined(USE_HIDAPI) - - // Initialize the hidapi library. - rc = hid_init(); - if (rc < 0) { - ERROR (context, "Failed to initialize usb support."); - status = DC_STATUS_IO; - goto error_free; - } - // Open the USB device. usbhid->handle = hid_open (vid, pid, NULL); if (usbhid->handle == NULL) { ERROR (context, "Failed to open the usb device."); status = DC_STATUS_IO; - goto error_hid_exit; + goto error_usb_exit; } usbhid->timeout = -1; @@ -274,12 +314,9 @@ error_usb_free_config: libusb_free_config_descriptor (config); error_usb_free_list: libusb_free_device_list (devices, 1); -error_usb_exit: - libusb_exit (usbhid->ctx); -#elif defined(USE_HIDAPI) -error_hid_exit: - hid_exit (); #endif +error_usb_exit: + dc_usbhid_exit (); error_free: free (usbhid); return status; @@ -300,11 +337,10 @@ dc_usbhid_close (dc_usbhid_t *usbhid) #if defined(USE_LIBUSB) libusb_release_interface (usbhid->handle, usbhid->interface); libusb_close (usbhid->handle); - libusb_exit (usbhid->ctx); #elif defined(USE_HIDAPI) hid_close(usbhid->handle); - hid_exit(); #endif + dc_usbhid_exit(); free (usbhid); return status; From f708eadcfdcbcea8a83c24ccd069b0955d5710d2 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 26 Sep 2017 22:58:59 +0200 Subject: [PATCH 2/2] Make the initialization thread-safe Perform the initialization inside a critical section. Unfortunately Windows critical sections, which are the simplest synchronization mechanism available on Windows, do not support static initialization. A call to InitializeCriticalSection is required. Therefore a simple spinlock, with an implementation based on atomic operations, is used as a workaround. --- configure.ac | 1 + src/usbhid.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/configure.ac b/configure.ac index 17bd5f9..d0939b9 100644 --- a/configure.ac +++ b/configure.ac @@ -154,6 +154,7 @@ AC_CHECK_HEADERS([linux/serial.h]) AC_CHECK_HEADERS([IOKit/serial/ioss.h]) AC_CHECK_HEADERS([getopt.h]) AC_CHECK_HEADERS([sys/param.h]) +AC_CHECK_HEADERS([pthread.h]) # Checks for global variable declarations. AC_CHECK_DECLS([optreset]) diff --git a/src/usbhid.c b/src/usbhid.c index 0f7ba7b..f0c10df 100644 --- a/src/usbhid.c +++ b/src/usbhid.c @@ -24,6 +24,13 @@ #endif #include +#ifdef HAVE_PTHREAD_H +#include +#endif +#ifdef _WIN32 +#define NOGDI +#include +#endif #if defined(HAVE_HIDAPI) #define USE_HIDAPI @@ -47,6 +54,14 @@ #include "context-private.h" #include "platform.h" +#ifdef _WIN32 +typedef LONG dc_mutex_t; +#define DC_MUTEX_INIT 0 +#else +typedef pthread_mutex_t dc_mutex_t; +#define DC_MUTEX_INIT PTHREAD_MUTEX_INITIALIZER +#endif + struct dc_usbhid_t { /* Library context. */ dc_context_t *context; @@ -87,16 +102,41 @@ syserror(int errcode) #endif #ifdef USBHID +static dc_mutex_t g_usbhid_mutex = DC_MUTEX_INIT; static size_t g_usbhid_refcount = 0; #ifdef USE_LIBUSB static libusb_context *g_usbhid_ctx = NULL; #endif +static void +dc_mutex_lock (dc_mutex_t *mutex) +{ +#ifdef _WIN32 + while (InterlockedCompareExchange (mutex, 1, 0) == 1) { + SleepEx (0, TRUE); + } +#else + pthread_mutex_lock (mutex); +#endif +} + +static void +dc_mutex_unlock (dc_mutex_t *mutex) +{ +#ifdef _WIN32 + InterlockedExchange (mutex, 0); +#else + pthread_mutex_unlock (mutex); +#endif +} + static dc_status_t dc_usbhid_init (dc_context_t *context) { dc_status_t status = DC_STATUS_SUCCESS; + dc_mutex_lock (&g_usbhid_mutex); + if (g_usbhid_refcount == 0) { #if defined(USE_LIBUSB) int rc = libusb_init (&g_usbhid_ctx); @@ -119,12 +159,14 @@ dc_usbhid_init (dc_context_t *context) g_usbhid_refcount++; error: + dc_mutex_unlock (&g_usbhid_mutex); return status; } static dc_status_t dc_usbhid_exit (void) { + dc_mutex_lock (&g_usbhid_mutex); if (--g_usbhid_refcount == 0) { #if defined(USE_LIBUSB) @@ -135,6 +177,8 @@ dc_usbhid_exit (void) #endif } + dc_mutex_unlock (&g_usbhid_mutex); + return DC_STATUS_SUCCESS; } #endif