Repository navigation
feat(libSceUsbd): handle events with FreeBSD libusb's event lock and handler thread - #9339
Conversation
Progress report (04fefeb → 9f3799c)📈 System libraries: 46.73% (+0.07%, +9 implemented functions) ✅ 9 implemented
Changed libraries (implemented exports)
|
|
Device run of #9339's head
Both runs print AI-assisted: yes |
|
Linux data point for
AI-assisted (Claude Code). |
|
@MaxLikesCode GuestUsbd.cpp:176 takes the event mutex, HandleEventsLocked at :188 reacquires it, and follower.join at :189 runs before unlock at :191. If the follower’s condition-variable timeout expires but its mutex reacquisition is delayed, it cannot finish while the main thread waits in join. Could you release the event mutex while establishing follower completion, retain the timeout assertion, and add a bounded regression that forces delayed reacquisition? The 100 ms/30 ms timing difference does not establish that ordering. |
|
@oneandonlydean thanks, that ordering could deadlock. The main thread joined the follower while holding the event mutex, so a follower whose 30 ms wait had timed out, but which had not yet reacquired the mutex, could not return. bda456a changes the test:
AI-assisted: yes (Claude Code) |
bda456a to
6c20825
Compare
|
Windows run and one review point for head Runs on this merge
Review point: waiters only wake when the handler unlocks FreeBSD's Measured on this merge with a local program (not part of the PR):
With that line, Not run: Linux, macOS, a console or title, the rest of the suite. AI-assisted: yes |
…handler thread sceUsbdLockEvents, sceUsbdTryLockEvents, sceUsbdUnlockEvents, sceUsbdLockEventWaiters, sceUsbdUnlockEventWaiters, sceUsbdWaitForEvent, sceUsbdEventHandlerActive, sceUsbdHandleEvents and sceUsbdHandleEventsLocked follow FreeBSD 11.4 libusb10_io.c: one lock for events and their waiters, one handler thread that releases the lock while the host libusb handles events, and waiters that are woken when the handler unlocks and take over handling when there is none. sceUsbdEventHandlingOk now reports whether the calling thread is the handler, and sceUsbdHandleEventsTimeout takes the same path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ishes The timed-out follower case joined the follower while the main thread held the event lock, so a follower whose timeout had expired but which had not yet reacquired the lock could not return. Keep handling events in slices, which release the lock, until the follower has returned, and run the case a second time with the lock held past the follower's timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FreeBSD 11.4's libusb10_handle_events_sub ends every pass, a timed-out one included, with pthread_cond_broadcast on the context's condition variable, so a thread in libusb_wait_for_event wakes after each pass of the handler. Here waiters woke only when the handler unlocked, so a waiter could sleep through the handler's whole loop. Notify them when the handler takes the event lock back after a pass. The follower test that expected a timeout while the handler ran 20 ms passes now races that wake-up, so guest_usbd checks three cases instead: a follower with a long timeout returns 0 after a pass, a follower times out during a longer pass, and a follower returns after the handler held the event lock past its timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6c20825 to
9f3799c
Compare
|
Thanks @simnJS for the runs and the review point; you were right. FreeBSD's As you found,
With the change, AI-assisted: yes (Claude Code) |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
What
Implements the nine remaining
sceUsbdplaceholders, the event calls, with the locking model of FreeBSD 11.4libusb10_io.cover the host libusb:sceUsbdLockEvents,sceUsbdTryLockEvents,sceUsbdUnlockEvents,sceUsbdLockEventWaiters,sceUsbdUnlockEventWaiters,sceUsbdWaitForEvent,sceUsbdEventHandlerActive,sceUsbdHandleEventsandsceUsbdHandleEventsLocked. Their signatures were already in the file, from orbital's console type database.FreeBSD's model, which upstream libusb (the host's) does not share:
sceUsbdEventHandlingOkreports whether the caller is the handler, andsceUsbdEventHandlerActivewhether any thread is.sceUsbdTryLockEventsreturns 1 when the lock is busy, held by the caller (FreeBSD'spthread_mutex_trylockgivesEBUSYthere) or when another thread handles events; otherwise it takes the lock and the handler role and returns 0.sceUsbdUnlockEventsgives up the handler role, wakes all waiters and releases the lock.sceUsbdWaitForEventwaits on that wake-up with the lock held, returns 1 on timeout and 0 otherwise, and takes the handler role if no thread has it.sceUsbdHandleEventsLockedhandles events if the caller is the handler, releasing the lock while the host libusb handles them and runs the transfer callbacks. Otherwise it waits and returns0x80240007(timeout) when the wait times out.libusb10_handle_events_subdoes withpthread_cond_broadcast(L223-L224); a waiter insceUsbdWaitForEventorsceUsbdHandleEventsLockedthen returns 0.sceUsbdHandleEventslocks, handles events without a timeout and unlocks.Changes to existing calls:
sceUsbdEventHandlingOkreturned 1 for every thread. It now reports the handler role, as FreeBSD'slibusb_event_handling_okdoes, and the existing check inguest_usbdexpects 0 from a thread without the lock.sceUsbdHandleEventsTimeoutnow takes the same lock and handler path, so a second thread handling events waits for the first instead of entering the host libusb at the same time. Its argument checks are unchanged, and a timeout of more thanINT_MAXseconds now throws.These throw, as FreeBSD's result there is undefined or unknown:
sceUsbdHandleEventsLockedwithout holding it;sceUsbdWaitForEventandsceUsbdHandleEventsLocked.With no timeout, FreeBSD's
pollreturns on any event of its descriptors, including the wake-up a submit from another thread writes. Here event handling without a timeout returns once a transfer completes, in one-second host slices. Callers loop on these calls, but the difference is recorded.docs/dev/debt/libSceUsbd.mddrops "sceUsbdEventHandlingOkreturns 1" and the empty unknown section, and adds one entry for the event calls. The library's 2214-character first entry is split into three entries of under 1000 characters with the same text, except that transfer callbacks now run "from event handling" rather than only fromsceUsbdHandleEventsTimeout.Tested
Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, full Release build with
BUILD_TESTING=ON, this branch on #9190 and #9338, merged ontomainb00235d.guest_usbdgainsEventLocks, which runs without a device:sceUsbdHandleEventsLockedwhen a pass of the handler ends, gets0x80240007when its timeout ends during a longer pass, and returns after the handler keeps the event lock past its timeout. The handler releases the lock until that thread has returned, and only then joins it;It passes 3 of 3, and 56 of 56 with 8 copies at once (rechecked at bda456a). With
Export.cppfrom the device calls PR and the new tests, it fails at the first changed answer (a thread without the event lock may handle events). With the earlier ordering, where the handler joined that thread while holding the event lock, the held-lock case hangs (3 of 3 runs stopped at 20 s).guest_usbd_tests devicewithAPS5_USBD_DEVICE=1b1c:0c1a(CORSAIR Lighting Node CORE) completes an asynchronous control transfer throughsceUsbdHandleEvents, and another undersceUsbdLockEventsthroughsceUsbdHandleEventsLocked, besides the existing transfer and cancel-race checks throughsceUsbdHandleEventsTimeout. It passes 11 of 11 runs.python tools/select_tests.py ctest build --output-on-failure --timeout 180 --parallel 8: 1523 of the 1526 selected tests pass. The 3 failures areagc_driver_sample_order,agc_driver_sample_order_grid_referenceandagc_driver_sample_order_recipe_depth_locations, which fail the same way onmainon this RTX 4090 and which test(agc): read the sample order tests' depth through a sampled copy #9056 fixes.agc_driver_image_array_window_past_layer_limitis not built: it does not compile with MinGW onmain(test(agc): align the array window test's texels at run time #8310).python tools/check_conventions.py --base origin/mainpasses.python tools/pr_overlap.py --branch feat/usbd-event-callsreports only feat(libSceUsbd): pass configuration and descriptor queries through to libusb #9190, which this builds on. test(libSceUsbd): cover sceUsbdFillControlSetup buffer formatting #9116 adds test lines toGuestUsbd.cpptoo; the two merge cleanly.guest_usbd_tests devicefrom feat(libSceUsbd): handle events with FreeBSD libusb's event lock and handler thread #9339's branch, which contains feat(libSceUsbd): pass the device, packet size, alternate setting and halt calls through to libusb #9338, on Windows 11 Pro 26200 (comment): an MSI Mystic Light HID controller (1462:7D75, full speed) and a Wooting 80HE keyboard (31E3:1402, high speed) pass 3 of 3 each, andsceUsbdDetachKernelDriverreturns0x802400FFon both. On the Wooting the iso packet size check takes the multiplier path above full speed.guest_usbd_testson Linux (WSL2, Ubuntu 24.04, GCC 13.3, Release) from this head merged ontomaindf2569d (comment): it exits with 0, and withExport.cppreset tomainit aborts (SIGABRT).guest_usbdpasses 3 of 3, and 56 of 56 with 8 copies at once, and fails 3 of 3 withExport.cppfrom the device calls PR. The device run withAPS5_USBD_DEVICE=1b1c:0c1apasses 2 of 2.public_headersand the conventions check pass.main750cf6c (Windows 11 Pro 26200, Ryzen 9 7950X3D, WinLibs GCC 15.2.0 r7, Release; comment):guest_usbd20 of 20;guest_usbd_tests device5 of 5 on1462:7D75and 5 of 5 on31E3:1402; a Logitech LIGHTSPEED receiver046D:C54Dpasses the newsceUsbdHandleEventsandsceUsbdHandleEventsLockedtransfers and stops at the existing cancel-race check, as it does withmain'sExport.cppandGuestUsbd.cpp;guest_usb_storage,guest_system_service_usb_storageandguest_usb_storage_dialog5 of 5 each. They also found that waiters woke only at the handler's unlock, where FreeBSD wakes them after each pass.maind5ef780 (the two earlier commits are now dbb27b9 and c0d8f0a): the handler wakes waiters after each pass, andguest_usbd's follower case is split into the three cases above. With the new test andExport.cppwithout the change,guest_usbdfails 3 of 3 ata thread that is not the handler was not woken when a pass of event handling ended(the follower returns only at its 5 s timeout). With the change it passes 10 of 10, and 56 of 56 with 8 copies at once.guest_usbd_tests devicewithAPS5_USBD_DEVICE=1b1c:0c1apasses 3 of 3. The full suite passes 2827 of 2828; onlyguest_wide_formattingfails, the same way as onmainon this machine (fix(libc): convert the Windows time zone name from the ANSI code page to UTF-8 for tzname, %Z and tm_zone #10981, German code page, fixed by fix(libc): convert Windows time zone names to UTF-8 #10987).python tools/check_conventions.py --base origin/mainpasses.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code