If I increase the log level to Error (apparently Notice silences all errors and only prints state-change events?), I see the following messages:
35:18:717 InputCommon/GCAdapter.cpp:211 E[CI]: Read: libusb_interrupt_transfer failed: Input/Output Error (-1: LIBUSB_ERROR_IO)
35:18:717 InputCommon/GCAdapter.cpp:775 E[CI]: error reading payload (size: 0, type: 21)
35:18:717 InputCommon/GCAdapter.cpp:211 E[CI]: Read: libusb_interrupt_transfer failed: Input/Output Error (-1: LIBUSB_ERROR_IO)
35:18:717 InputCommon/GCAdapter.cpp:775 E[CI]: error reading payload (size: 0, type: 21)
...
If I instead unplug the adapter, I see the following:
37:46:466 InputCommon/GCAdapter.cpp:211 E[CI]: Read: libusb_interrupt_transfer failed: No such device (it may have been disconnected) (-4: LIBUSB_ERROR_NO_DEVICE)
37:46:466 InputCommon/GCAdapter.cpp:775 E[CI]: error reading payload (size: 0, type: 21)
37:46:466 InputCommon/GCAdapter.cpp:211 E[CI]: Read: libusb_interrupt_transfer failed: No such device (it may have been disconnected) (-4: LIBUSB_ERROR_NO_DEVICE)
37:46:466 InputCommon/GCAdapter.cpp:775 E[CI]: error reading payload (size: 0, type: 21)
37:46:467 InputCommon/GCAdapter.cpp:211 E[CI]: Read: libusb_interrupt_transfer failed: No such device (it may have been disconnected) (-4: LIBUSB_ERROR_NO_DEVICE)
37:46:467 InputCommon/GCAdapter.cpp:775 E[CI]: error reading payload (size: 0, type: 21)
37:46:467 InputCommon/GCAdapter.cpp:297 N[CI]: GCAdapter write thread stopped
37:46:467 InputCommon/GCAdapter.cpp:249 N[CI]: GCAdapter read thread stopped
37:46:467 InputCommon/GCAdapter.cpp:724 N[CI]: GC Adapter detached
and the errors do not continue forever.
(hard) Changing control flow¶
During setup, you call libusb_hotplug_register_callback
and pass in function HotplugCallback
. When the adapter is unplugged, HotplugCallback
is called which tears down the adapter.
When you sleep-wake, HotplugCallback
is never called and libusb_interrupt_transfer
instead fails with -1 LIBUSB_ERROR_IO
. How should we handle this?
https://libusb.sourceforge.io/api-1.0/group__libusb__syncio.html#ga0f171a72904a552fc43e6e6564d108a3 mentions:
You should also check the transferred
parameter for interrupt writes. Not all of the data may have been written.
Also check transferred
when dealing with a timeout error code. libusb may have to split your transfer into a number of chunks to satisfy underlying O/S requirements, meaning that the timeout may expire after the first few chunks have completed. libusb is careful not to lose any data that may have been transferred; do not assume that timeout conditions indicate a complete lack of I/O. See Timeouts for more details.
We don't seem to check the "actual length" of the transfer. Is that a problem? Who knows, but it's not the cause of the sleep-wake issue. And we still need to properly handle LIBUSB_ERROR_IO
.
Logically it would make sense to call Reset()
when ReadThreadFunc()
fails. But Reset()
joins the s_read_adapter_thread
handle, which is nonsensical to do on the thread itself. If we don't want to rewrite the relevant subset of Reset()
to be callable on ReadThreadFunc()
itself, we'd have to do something like make Context::Impl::EventThread()
check not only libusb_handle_events_timeout_completed()
(which ignores devices going invalid), but polling on a dup of a "should reset" eventfd signalled by ReadThreadFunc() when it's exiting prematurely... oh god this code needs to work on Windows too...
(easier) Fixing the existing libusb_device_handle
If you don't want to reset the whole thing, I noticed that libusb_kernel_driver_active()
is once again set to 1 after sleep-wake and libusb_interrupt_transfer() == LIBUSB_ERROR_IO
. So we could factor out the "libusb_detach_kernel_driver() if needed" code into a separate function, and call it upon LIBUSB_ERROR_IO
, and if it fails kill the GC thread (no GC input is better than burning a CPU core and flooding the Linux kernel log). I've mocked up an implementation at https://github.com/nyanpasu64/dolphin/commits/gc-adapter-sleep-detach (the diffs are correct, but this branch is based on an older revision and has other unrelated changes as well).