From 17dddeedc43c844f0df3a36db859c4c8cd7a833c Mon Sep 17 00:00:00 2001 From: Donny Lawrence Date: Wed, 15 Jan 2020 15:55:36 -0600 Subject: [PATCH] device: Fix 10.15 segfault on USB hotplug For some reason, IOHIDDeviceRegisterRemovalCallback() no longer works on 10.15+, so an app will crash once trying to poll a device that doesn't exist anymore. Thankfully, there is the alternative solution of using IOHIDManagerRegisterDeviceRemovalCallback(). This just required a little rearranging of the callback code, as well as keeping track of the connection between IOHIDDeviceRefs and IOKitInputDevices so we actually know which device to remove. Closes #847 --- panda/src/device/ioKitInputDevice.cxx | 35 -------------------- panda/src/device/ioKitInputDevice.h | 2 -- panda/src/device/ioKitInputDeviceManager.cxx | 32 ++++++++++++++++-- panda/src/device/ioKitInputDeviceManager.h | 11 ++++++ 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/panda/src/device/ioKitInputDevice.cxx b/panda/src/device/ioKitInputDevice.cxx index 1d8bf38f0f..302f3c1d5b 100644 --- a/panda/src/device/ioKitInputDevice.cxx +++ b/panda/src/device/ioKitInputDevice.cxx @@ -21,15 +21,6 @@ #include "gamepadButton.h" #include "mouseButton.h" -static void removal_callback(void *ctx, IOReturn result, void *sender) { - // We need to hold a reference to this because it may otherwise be destroyed - // during the call to on_remove(). - PT(IOKitInputDevice) input_device = (IOKitInputDevice *)ctx; - nassertv(input_device != nullptr); - nassertv(input_device->test_ref_count_integrity()); - input_device->on_remove(); -} - /** * Protected constructor. */ @@ -137,7 +128,6 @@ IOKitInputDevice(IOHIDDeviceRef device) : } _is_connected = true; - IOHIDDeviceRegisterRemovalCallback(device, removal_callback, this); } /** @@ -147,31 +137,6 @@ IOKitInputDevice:: ~IOKitInputDevice() { } -/** - * The nonstatic version of on_remove_device. - */ -void IOKitInputDevice:: -on_remove() { - { - LightMutexHolder holder(_lock); - if (!_is_connected) { - return; - } - _is_connected = false; - } - - if (device_cat.is_debug()) { - device_cat.debug() - << "Removed input device " << *this << "\n"; - } - - IOHIDDeviceClose(_device, kIOHIDOptionsTypeNone); - - InputDeviceManager *mgr = InputDeviceManager::get_global_ptr(); - nassertv(mgr != nullptr); - mgr->remove_device(this); -} - /** * */ diff --git a/panda/src/device/ioKitInputDevice.h b/panda/src/device/ioKitInputDevice.h index f32f9c4256..e355a95826 100644 --- a/panda/src/device/ioKitInputDevice.h +++ b/panda/src/device/ioKitInputDevice.h @@ -30,8 +30,6 @@ public: IOKitInputDevice(IOHIDDeviceRef device); ~IOKitInputDevice(); - void on_remove(); - private: void parse_element(IOHIDElementRef element); diff --git a/panda/src/device/ioKitInputDeviceManager.cxx b/panda/src/device/ioKitInputDeviceManager.cxx index 6874d9baba..3f1c807469 100644 --- a/panda/src/device/ioKitInputDeviceManager.cxx +++ b/panda/src/device/ioKitInputDeviceManager.cxx @@ -59,6 +59,7 @@ IOKitInputDeviceManager() { CFRelease(match); IOHIDManagerRegisterDeviceMatchingCallback(_hid_manager, on_match_device, this); + IOHIDManagerRegisterDeviceRemovalCallback(_hid_manager, on_remove_device, this); IOHIDManagerScheduleWithRunLoop(_hid_manager, CFRunLoopGetMain(), kCFRunLoopCommonModes); IOHIDManagerOpen(_hid_manager, kIOHIDOptionsTypeNone); } @@ -78,16 +79,43 @@ IOKitInputDeviceManager:: */ void IOKitInputDeviceManager:: on_match_device(void *ctx, IOReturn result, void *sender, IOHIDDeviceRef device) { - InputDeviceManager *mgr = (InputDeviceManager *)ctx; + IOKitInputDeviceManager *mgr = (IOKitInputDeviceManager *)ctx; nassertv(mgr != nullptr); nassertv(device); - PT(InputDevice) input_device = new IOKitInputDevice(device); + IOKitInputDevice *input_device = new IOKitInputDevice(device); if (device_cat.is_debug()) { device_cat.debug() << "Discovered input device " << *input_device << "\n"; } mgr->add_device(input_device); + mgr->_devices_by_hidref[device] = input_device; } +/** + * Called by IOKit when an input device has disappeared. + */ +void IOKitInputDeviceManager:: +on_remove_device(void *ctx, IOReturn result, void *sender, IOHIDDeviceRef device) { + IOKitInputDeviceManager *mgr = (IOKitInputDeviceManager *)ctx; + nassertv(mgr != nullptr); + nassertv(device); + + auto it = mgr->_devices_by_hidref.find(device); + nassertv(it != mgr->_devices_by_hidref.end()); + IOKitInputDevice *input_device = it->second; + + input_device->set_connected(false); + + mgr->_devices_by_hidref.erase(device); + + IOHIDDeviceClose(device, kIOHIDOptionsTypeNone); + + if (device_cat.is_debug()) { + device_cat.debug() + << "Removed input device " << *input_device << "\n"; + } + + mgr->remove_device(input_device); +} #endif diff --git a/panda/src/device/ioKitInputDeviceManager.h b/panda/src/device/ioKitInputDeviceManager.h index f028a5f153..80005f99ab 100644 --- a/panda/src/device/ioKitInputDeviceManager.h +++ b/panda/src/device/ioKitInputDeviceManager.h @@ -19,6 +19,8 @@ #if defined(__APPLE__) && !defined(CPPPARSER) #include +class IOKitInputDevice; + /** * The macOS implementation of InputDeviceManager. */ @@ -30,7 +32,16 @@ protected: protected: IOHIDManagerRef _hid_manager; + // The device removal callback method we need to use requires us to remember + // which IOKitInputDevice corresponds to which IOHIDDeviceRef. This is the + // same strategy used by winInputDevice and friends. + // + // We can make this a mapping to raw pointers since we know _devices will be + // holding a reference until remove_device is called. + pmap _devices_by_hidref; + static void on_match_device(void *ctx, IOReturn result, void *sender, IOHIDDeviceRef device); + static void on_remove_device(void *ctx, IOReturn result, void *sender, IOHIDDeviceRef device); friend class InputDeviceManager; };