From 21e1101dbc468700996e0f32e21a346deb009cb3 Mon Sep 17 00:00:00 2001 From: Dmitry Kychanov Date: Sat, 9 Mar 2024 10:29:54 +0400 Subject: [PATCH] Attempt to fix event thread --- hidtest/test.c | 39 ++++++------ mac/hid.c | 157 +++++++++++++++++++++++++++++++++++++------------ 2 files changed, 141 insertions(+), 55 deletions(-) diff --git a/hidtest/test.c b/hidtest/test.c index dd11ab9e0..b398332c1 100644 --- a/hidtest/test.c +++ b/hidtest/test.c @@ -117,24 +117,27 @@ int device_callback( { (void)user_data; - if (event & HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED) - printf("Handle %d: New device is connected: %s.\n", callback_handle, device->path); - else - printf("Handle %d: Device was disconnected: %s.\n", callback_handle, device->path); - - printf("type: %04hx %04hx\n serial_number: %ls", device->vendor_id, device->product_id, device->serial_number); - printf("\n"); - printf(" Manufacturer: %ls\n", device->manufacturer_string); - printf(" Product: %ls\n", device->product_string); - printf(" Release: %hx\n", device->release_number); - printf(" Interface: %d\n", device->interface_number); - printf(" Usage (page): 0x%hx (0x%hx)\n", device->usage, device->usage_page); - printf("\n"); - - //if (device->product_id == 0x0ce6) - // return 1; - - return 0; + if (event & HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED) + printf("Handle %d: New device is connected: %s.\n", callback_handle, device->path); + else + printf("Handle %d: Device was disconnected: %s.\n", callback_handle, device->path); + + printf("type: %04hx %04hx\n serial_number: %ls", device->vendor_id, device->product_id, device->serial_number); + printf("\n"); + printf(" Manufacturer: %ls\n", device->manufacturer_string); + printf(" Product: %ls\n", device->product_string); + printf(" Release: %hx\n", device->release_number); + printf(" Interface: %d\n", device->interface_number); + printf(" Usage (page): 0x%hx (0x%hx)\n", device->usage, device->usage_page); + printf("\n"); + + //if (device->product_id == 0x0ce6) + // return 1; + + /* Printed data might not show on the screen - force it out */ + fflush(stdout); + + return 0; } int main(int argc, char* argv[]) diff --git a/mac/hid.c b/mac/hid.c index 201cbb894..2c1f24405 100644 --- a/mac/hid.c +++ b/mac/hid.c @@ -447,6 +447,14 @@ static struct hid_hotplug_context { /* MacOS specific notification handles */ IOHIDManagerRef manager; + /* Thread and RunLoop for the manager to work in */ + pthread_t thread; + CFRunLoopRef run_loop; + CFRunLoopSourceRef source; + CFStringRef run_loop_mode; + pthread_barrier_t startup_barrier; /* Ensures correct startup sequence */ + int thread state; + /* HIDAPI unique callback handle counter */ hid_hotplug_callback_handle next_handle; @@ -461,8 +469,12 @@ static struct hid_hotplug_context { struct hid_device_info *devs; } hid_hotplug_context = { .manager = NULL, + .run_loop = NULL, + .run_loop_mode = NULL, + .source = NULL, .next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE, .mutex_ready = 0, + .thread_state = 0, /* 0 = starting (events ignored), 1 = running (events processed), 2 = shutting down */ .hotplug_cbs = NULL, .devs = NULL }; @@ -472,14 +484,20 @@ static void hid_internal_hotplug_cleanup() if (hid_hotplug_context.hotplug_cbs != NULL) { return; } + /* Cleanup connected device list */ hid_free_enumeration(hid_hotplug_context.devs); hid_hotplug_context.devs = NULL; - - /* Kill the manager */ - IOHIDManagerClose(hid_hotplug_context.manager, kIOHIDOptionsTypeNone); - CFRelease(hid_hotplug_context.manager); - hid_hotplug_context.manager = NULL; + + /* Cause hotplug_thread() to stop. */ + hid_hotplug_context.thread_state = 2; + + /* Wake up the run thread's event loop so that the thread can exit. */ + CFRunLoopSourceSignal(hid_hotplug_context.source); + CFRunLoopWakeUp(hid_hotplug_context.run_loop); + + /* Wait for read_thread() to end. */ + pthread_join(hid_hotplug_context.thread, NULL); } static void hid_internal_hotplug_init() @@ -854,16 +872,20 @@ static void hid_internal_hotplug_connect_callback(void *context, IOReturn result (void) sender; struct hid_device_info* info = create_device_info(device); - - /* Lock the mutex to avoid race conditions */ - pthread_mutex_lock(&hid_hotplug_context.mutex); - - /* Invoke all callbacks */ struct hid_device_info* info_cur = info; - while(info_cur) + + /* NOTE: we don't call any callbacks and we don't lock the mutex during initialization: the mutex is held by the main thread, but it's waiting by a barrier*/ + if (hid_hotplug_context.thread_state > 0) { - hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED); - info_cur = info_cur->next; + /* Lock the mutex to avoid race conditions */ + pthread_mutex_lock(&hid_hotplug_context.mutex); + + /* Invoke all callbacks */ + while(info_cur) + { + hid_internal_invoke_callbacks(info_cur, HID_API_HOTPLUG_EVENT_DEVICE_ARRIVED); + info_cur = info_cur->next; + } } /* Append all we got to the end of the device list */ @@ -880,7 +902,10 @@ static void hid_internal_hotplug_connect_callback(void *context, IOReturn result } } - pthread_mutex_unlock(&hid_hotplug_context.mutex); + if (hid_hotplug_context.thread_state > 0) + { + pthread_mutex_unlock(&hid_hotplug_context.mutex); + } } int match_ref_to_info(IOHIDDeviceRef device, struct hid_device_info *info) @@ -906,7 +931,11 @@ static void hid_internal_hotplug_disconnect_callback(void *context, IOReturn res (void) result; (void) sender; - pthread_mutex_lock(&hid_hotplug_context.mutex); + /* NOTE: we don't call any callbacks and we don't lock the mutex during initialization: the mutex is held by the main thread, but it's waiting by a barrier*/ + if (hid_hotplug_context.thread_state > 0) + { + pthread_mutex_lock(&hid_hotplug_context.mutex); + } for (struct hid_device_info **current = &hid_hotplug_context.devs; *current;) { struct hid_device_info* info = *current; @@ -921,11 +950,80 @@ static void hid_internal_hotplug_disconnect_callback(void *context, IOReturn res current = &info->next; } } + + if (hid_hotplug_context.thread_state > 0) + { + /* Clean up if the last callback was removed */ + hid_internal_hotplug_cleanup(); + pthread_mutex_unlock(&hid_hotplug_context.mutex); + } +} + +static void* hotplug_thread(void* user_data) +{ + hid_hotplug_context.thread_state = 0; + hid_hotplug_context.manager = IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone); + + const char *str = "HIDAPI_hotplug"; + hid_hotplug_context.run_loop_mode = CFStringCreateWithCString(NULL, str, kCFStringEncodingASCII); + + /* Ensure the manager runs in this thread */ + IOHIDManagerScheduleWithRunLoop(hid_hotplug_context.manager, CFRunLoopGetCurrent(), hid_hotplug_context.run_loop_mode); + /* Store a reference to this runloop if we ever need to stop it - e.g. if we have no callbacks left or hid_exit was called */ + hid_hotplug_context.run_loop = CFRunLoopGetCurrent(); + + /* Create the RunLoopSource which is used to signal the + event loop to stop when hid_close() is called. */ + CFRunLoopSourceContext ctx; + memset(&ctx, 0, sizeof(ctx)); + ctx.version = 0; + ctx.info = dev; + ctx.perform = &perform_signal_callback; + hid_hotplug_context.source = CFRunLoopSourceCreate(kCFAllocatorDefault, 0/*order*/, &ctx); + CFRunLoopAddSource(hid_hotplug_context.run_loop, hid_hotplug_context.source, hid_hotplug_context.run_loop_mode); + + /* Set the manager to receive events for ALL HID devices */ + IOHIDManagerSetDeviceMatching(hid_hotplug_context.manager, NULL); + + /* Install callbacks */ + IOHIDManagerRegisterDeviceMatchingCallback(hid_hotplug_context.manager, + hid_internal_hotplug_connect_callback, + NULL); + + IOHIDManagerRegisterDeviceRemovalCallback(hid_hotplug_context.manager, + hid_internal_hotplug_disconnect_callback, + NULL); + /* After monitoring is all set up, enumerate all devices */ + /* Opening the manager should result in the internal callback being called for all connected devices */ + IOHIDManagerOpen(hid_hotplug_context.manager, kIOHIDOptionsTypeNone); - /* Clean up if the last callback was removed */ - hid_internal_hotplug_cleanup(); - pthread_mutex_unlock(&hid_hotplug_context.mutex); + /* TODO: We need to flush all events from the runloop to ensure the already connected devices don't send any unwanted events */ + process_pending_events(); + + /* Now that all events are flushed, we are ready to notify the main thread that we are ready */ + hid_hotplug_context.thread_state = 1; + pthread_barrier_wait(&hid_hotplug_context.startup_barrier); + + while (hid_hotplug_context.thread_state != 2) { + code = CFRunLoopRunInMode(hid_hotplug_context.run_loop_mode, 1000/*sec*/, FALSE); + /* If runloop stopped for whatever reason, exit the thread */ + if (code != kCFRunLoopRunTimedOut && + code != kCFRunLoopRunHandledSource) { + hid_hotplug_context.thread_state = 2; + break; + } + } + + /* Kill the manager */ + IOHIDManagerClose(hid_hotplug_context.manager, kIOHIDOptionsTypeNone); + + IOHIDManagerUnscheduleFromRunLoop(hid_hotplug_context.manager, hid_hotplug_context.run_loop, hid_hotplug_context.run_loop_mode); + + CFRelease(hid_hotplug_context.manager); + hid_hotplug_context.manager = NULL; + + return NULL; } int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short vendor_id, unsigned short product_id, int events, int flags, hid_hotplug_callback_fn callback, void *user_data, hid_hotplug_callback_handle *callback_handle) @@ -982,28 +1080,13 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven last->next = hotplug_cb; } else { - /* Set up platform-dependant monitoring */ - hid_hotplug_context.manager = IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDOptionsTypeNone); - - IOHIDManagerSetDeviceMatching(hid_hotplug_context.manager, NULL); + pthread_barrier_init(&hid_hotplug_context.startup_barrier, NULL, 2); + pthread_create(&hid_hotplug_context.thread, NULL, hotplug_thread, NULL); - /* The manager requires a runloop (a background thread) to work in */ - IOHIDManagerScheduleWithRunLoop(hid_hotplug_context.manager, CFRunLoopGetCurrent(), kCFRunLoopDefaultMode); + /* Wait for the thread to finish setting up - without it the callback may be registered too early*/ - IOHIDManagerRegisterDeviceMatchingCallback(hid_hotplug_context.manager, - hid_internal_hotplug_connect_callback, - NULL); + pthread_barrier_wait(&hid_hotplug_context.startup_barrier); - IOHIDManagerRegisterDeviceRemovalCallback(hid_hotplug_context.manager, - hid_internal_hotplug_disconnect_callback, - NULL); - - /* After monitoring is all set up, enumerate all devices */ - /* Opening the manager should result in the internal callback being called for all connected devices */ - /* We avoid adding the requested callback to the list just yet in case HID_API_HOTPLUG_ENUMERATE is not set */ - IOHIDManagerOpen(hid_hotplug_context.manager, kIOHIDOptionsTypeNone); - //hid_hotplug_context.devs = hid_enumerate(0, 0); - /* Don't forget to actually register the callback */ hid_hotplug_context.hotplug_cbs = hotplug_cb; }