core: Introduce atomic type and operations
An atomic variable is useful for reference counting and is much less
overhead than accessing such a variable while holding a lock. To that
end, replace the libusb_device 'refcnt' variable with an atomic and use
the atomic operations to manipulate it. This removes the need for the
mutex in the libusb_device.
Also convert the 'attached' variable to an atomic as well. This variable
was previously accessed both while holding the libusb_device mutex and
not.
Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>
diff --git a/libusb/core.c b/libusb/core.c
index 07d459c..051b4f7 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -704,10 +704,9 @@
if (!dev)
return NULL;
- usbi_mutex_init(&dev->lock);
+ usbi_atomic_store(&dev->refcnt, 1);
dev->ctx = ctx;
- dev->refcnt = 1;
dev->session_data = session_id;
dev->speed = LIBUSB_SPEED_UNKNOWN;
@@ -722,7 +721,7 @@
{
struct libusb_context *ctx = DEVICE_CTX(dev);
- dev->attached = 1;
+ usbi_atomic_store(&dev->attached, 1);
usbi_mutex_lock(&dev->ctx->usb_devs_lock);
list_add(&dev->list, &dev->ctx->usb_devs);
@@ -740,9 +739,7 @@
{
struct libusb_context *ctx = DEVICE_CTX(dev);
- usbi_mutex_lock(&dev->lock);
- dev->attached = 0;
- usbi_mutex_unlock(&dev->lock);
+ usbi_atomic_store(&dev->attached, 0);
usbi_mutex_lock(&ctx->usb_devs_lock);
list_del(&dev->list);
@@ -1173,9 +1170,11 @@
DEFAULT_VISIBILITY
libusb_device * LIBUSB_CALL libusb_ref_device(libusb_device *dev)
{
- usbi_mutex_lock(&dev->lock);
- dev->refcnt++;
- usbi_mutex_unlock(&dev->lock);
+ long refcnt;
+
+ refcnt = usbi_atomic_inc(&dev->refcnt);
+ assert(refcnt >= 2);
+
return dev;
}
@@ -1186,14 +1185,13 @@
*/
void API_EXPORTED libusb_unref_device(libusb_device *dev)
{
- int refcnt;
+ long refcnt;
if (!dev)
return;
- usbi_mutex_lock(&dev->lock);
- refcnt = --dev->refcnt;
- usbi_mutex_unlock(&dev->lock);
+ refcnt = usbi_atomic_dec(&dev->refcnt);
+ assert(refcnt >= 0);
if (refcnt == 0) {
usbi_dbg("destroy device %d.%d", dev->bus_number, dev->device_address);
@@ -1208,7 +1206,6 @@
usbi_disconnect_device(dev);
}
- usbi_mutex_destroy(&dev->lock);
free(dev);
}
}
@@ -1306,11 +1303,11 @@
struct libusb_device_handle *_dev_handle;
size_t priv_size = usbi_backend.device_handle_priv_size;
int r;
+
usbi_dbg("open %d.%d", dev->bus_number, dev->device_address);
- if (!dev->attached) {
+ if (!usbi_atomic_load(&dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
- }
_dev_handle = calloc(1, PTR_ALIGN(sizeof(*_dev_handle)) + priv_size);
if (!_dev_handle)
@@ -1673,7 +1670,7 @@
if (interface_number < 0 || interface_number >= USB_MAXINTERFACES)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
usbi_mutex_lock(&dev_handle->lock);
@@ -1763,12 +1760,12 @@
if (alternate_setting < 0 || alternate_setting > (int)UINT8_MAX)
return LIBUSB_ERROR_INVALID_PARAM;
- usbi_mutex_lock(&dev_handle->lock);
- if (!dev_handle->dev->attached) {
+ if (!usbi_atomic_load(&dev_handle->dev->attached)) {
usbi_mutex_unlock(&dev_handle->lock);
return LIBUSB_ERROR_NO_DEVICE;
}
+ usbi_mutex_lock(&dev_handle->lock);
if (!(dev_handle->claimed_interfaces & (1U << interface_number))) {
usbi_mutex_unlock(&dev_handle->lock);
return LIBUSB_ERROR_NOT_FOUND;
@@ -1799,7 +1796,7 @@
unsigned char endpoint)
{
usbi_dbg("endpoint %x", endpoint);
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
return usbi_backend.clear_halt(dev_handle, endpoint);
@@ -1827,7 +1824,7 @@
int API_EXPORTED libusb_reset_device(libusb_device_handle *dev_handle)
{
usbi_dbg(" ");
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.reset_device)
@@ -1865,7 +1862,7 @@
if (!num_streams || !endpoints || num_endpoints <= 0)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.alloc_streams)
@@ -1895,7 +1892,7 @@
if (!endpoints || num_endpoints <= 0)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.free_streams)
@@ -1933,7 +1930,7 @@
unsigned char * LIBUSB_CALL libusb_dev_mem_alloc(libusb_device_handle *dev_handle,
size_t length)
{
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return NULL;
if (usbi_backend.dev_mem_alloc)
@@ -1984,7 +1981,7 @@
if (interface_number < 0 || interface_number >= USB_MAXINTERFACES)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.kernel_driver_active)
@@ -2022,7 +2019,7 @@
if (interface_number < 0 || interface_number >= USB_MAXINTERFACES)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.detach_kernel_driver)
@@ -2059,7 +2056,7 @@
if (interface_number < 0 || interface_number >= USB_MAXINTERFACES)
return LIBUSB_ERROR_INVALID_PARAM;
- if (!dev_handle->dev->attached)
+ if (!usbi_atomic_load(&dev_handle->dev->attached))
return LIBUSB_ERROR_NO_DEVICE;
if (usbi_backend.attach_kernel_driver)
@@ -2407,6 +2404,9 @@
list_del(&ctx->list);
usbi_mutex_static_unlock(&active_contexts_lock);
+ /* Don't bother with locking after this point because unless there is
+ * an application bug, nobody will be accessing these. */
+
if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG)) {
usbi_hotplug_deregister(ctx, 1);
@@ -2422,18 +2422,28 @@
if (list_empty(&ctx->open_devs))
libusb_handle_events_timeout(ctx, &tv);
- usbi_mutex_lock(&ctx->usb_devs_lock);
for_each_device_safe(ctx, dev, next) {
+ if (usbi_atomic_load(&dev->refcnt) > 1)
+ usbi_warn(ctx, "device %d.%d still referenced",
+ dev->bus_number, dev->device_address);
list_del(&dev->list);
libusb_unref_device(dev);
}
- usbi_mutex_unlock(&ctx->usb_devs_lock);
+ } else {
+ /*
+ * Backends without hotplug store enumerated devices on the
+ * usb_devs list when libusb_get_device_list() is called.
+ * These devices are removed from the list when the last
+ * reference is dropped, typically when the device list is
+ * freed. Any device still on the list has a reference held
+ * by the app, which is a bug.
+ */
+ for_each_device(ctx, dev) {
+ usbi_warn(ctx, "device %d.%d still referenced",
+ dev->bus_number, dev->device_address);
+ }
}
- /* a few sanity checks. don't bother with locking because unless
- * there is an application bug, nobody will be accessing these. */
- if (!list_empty(&ctx->usb_devs))
- usbi_warn(ctx, "some libusb_devices were leaked");
if (!list_empty(&ctx->open_devs))
usbi_warn(ctx, "application left some devices open");
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index 491114b..aea1bac 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -83,6 +83,33 @@
#define PTR_ALIGN(v) \
(((v) + (sizeof(void *) - 1)) & ~(sizeof(void *) - 1))
+/* Atomic operations
+ *
+ * Useful for reference counting or when accessing a value without a lock
+ *
+ * The following atomic operations are defined:
+ * usbi_atomic_load() - Atomically read a variable's value
+ * usbi_atomic_store() - Atomically write a new value value to a variable
+ * usbi_atomic_inc() - Atomically increment a variable's value and return the new value
+ * usbi_atomic_dec() - Atomically decrement a variable's value and return the new value
+ *
+ * All of these operations are ordered with each other, thus the effects of
+ * any one operation is guaranteed to be seen by any other operation.
+ */
+#ifdef _MSC_VER
+typedef volatile LONG usbi_atomic_t;
+#define usbi_atomic_load(a) (*(a))
+#define usbi_atomic_store(a, v) (*(a)) = (v)
+#define usbi_atomic_inc(a) InterlockedIncrement((a))
+#define usbi_atomic_dec(a) InterlockedDecrement((a))
+#else
+typedef long usbi_atomic_t;
+#define usbi_atomic_load(a) __atomic_load_n((a), __ATOMIC_SEQ_CST)
+#define usbi_atomic_store(a, v) __atomic_store_n((a), (v), __ATOMIC_SEQ_CST)
+#define usbi_atomic_inc(a) __atomic_add_fetch((a), 1, __ATOMIC_SEQ_CST)
+#define usbi_atomic_dec(a) __atomic_sub_fetch((a), 1, __ATOMIC_SEQ_CST)
+#endif
+
/* Internal abstractions for event handling and thread synchronization */
#if defined(PLATFORM_POSIX)
#include "os/events_posix.h"
@@ -450,10 +477,7 @@
}
struct libusb_device {
- /* lock protects refcnt, everything else is finalized at initialization
- * time */
- usbi_mutex_t lock;
- int refcnt;
+ usbi_atomic_t refcnt;
struct libusb_context *ctx;
struct libusb_device *parent_dev;
@@ -467,7 +491,7 @@
unsigned long session_data;
struct libusb_device_descriptor device_descriptor;
- int attached;
+ usbi_atomic_t attached;
};
struct libusb_device_handle {
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 4d2dc8d..7175b35 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1365,7 +1365,7 @@
goto out;
/* Consider the device as connected, but do not add it to the managed
* device list. */
- dev->attached = 1;
+ usbi_atomic_store(&dev->attached, 1);
handle->dev = dev;
r = initialize_handle(handle, fd);
@@ -1387,7 +1387,7 @@
/* device will still be marked as attached if hotplug monitor thread
* hasn't processed remove event yet */
usbi_mutex_static_lock(&linux_hotplug_lock);
- if (handle->dev->attached) {
+ if (usbi_atomic_load(&handle->dev->attached)) {
usbi_dbg("open failed with no device, but device still attached");
linux_device_disconnected(handle->dev->bus_number,
handle->dev->device_address);
@@ -2711,7 +2711,7 @@
/* device will still be marked as attached if hotplug monitor thread
* hasn't processed remove event yet */
usbi_mutex_static_lock(&linux_hotplug_lock);
- if (handle->dev->attached)
+ if (usbi_atomic_load(&handle->dev->attached))
linux_device_disconnected(handle->dev->bus_number,
handle->dev->device_address);
usbi_mutex_static_unlock(&linux_hotplug_lock);
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 578b097..6926971 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11586
+#define LIBUSB_NANO 11587
