From 9f3799cfaadcedffae53b0dea63ddb7ee57fe768 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 28 Oct 2026 15:27:07 +0100 Subject: [PATCH] fix(libSceUsbd): wake event waiters after each pass of event handling 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 --- core/libs/prx/libSceUsbd/Export.cpp | 1 + core/libs/tests/GuestUsbd.cpp | 60 +++++++++++++++++++++++++---- docs/dev/debt/libSceUsbd.md | 2 +- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/core/libs/prx/libSceUsbd/Export.cpp b/core/libs/prx/libSceUsbd/Export.cpp index 579df9557c..ea86f3130f 100644 --- a/core/libs/prx/libSceUsbd/Export.cpp +++ b/core/libs/prx/libSceUsbd/Export.cpp @@ -200,6 +200,7 @@ int HandleAsHandler(UsbdEvents& events, const UsbdTimeval* timeout) { } events.lock.lock(); events.owner.store(self); + events.changed.notify_all(); return result; } diff --git a/core/libs/tests/GuestUsbd.cpp b/core/libs/tests/GuestUsbd.cpp index 3c0320c75f..5fc00c0749 100644 --- a/core/libs/tests/GuestUsbd.cpp +++ b/core/libs/tests/GuestUsbd.cpp @@ -51,27 +51,70 @@ void APS5_VABI MarkDoneThenSignal(UsbdTransfer* transfer) { notified->store(true, std::memory_order_release); } -void FollowerTimesOut(bool holdPastTimeout) { +void FollowerWakesAfterPass() { UsbdTimeval brief{0, 20000}; sceUsbdLockEvents(); - std::atomic entered{false}; + std::atomic finished{false}; + std::atomic waiterResult{-1}; + std::atomic waited{0}; + std::thread follower([&] { + sceUsbdLockEventWaiters(); + UsbdTimeval patience{5, 0}; + const auto start = std::chrono::steady_clock::now(); + waiterResult.store(sceUsbdHandleEventsLocked(&patience)); + waited.store(std::chrono::duration_cast(std::chrono::steady_clock::now() - start).count()); + sceUsbdUnlockEventWaiters(); + finished.store(true); + }); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + while (!finished.load() && std::chrono::steady_clock::now() < deadline) Require(sceUsbdHandleEventsLocked(&brief) == 0, "the handler failed while another thread waited"); + Require(finished.load(), "a thread that is not the handler did not return from sceUsbdHandleEventsLocked while the handler released the event lock"); + follower.join(); + Require(waiterResult.load() == 0 && waited.load() < 2000, "a thread that is not the handler was not woken when a pass of event handling ended"); + sceUsbdUnlockEvents(); + Require(sceUsbdEventHandlerActive() == 0, "the handler stayed active at the end"); +} + +void FollowerTimesOutDuringPass() { + UsbdTimeval longPass{2, 0}; + sceUsbdLockEvents(); std::atomic finished{false}; std::atomic waiterResult{0}; std::thread follower([&] { sceUsbdLockEventWaiters(); - entered.store(true); UsbdTimeval shortWait{0, 30000}; waiterResult.store(sceUsbdHandleEventsLocked(&shortWait)); sceUsbdUnlockEventWaiters(); finished.store(true); }); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + while (!finished.load() && std::chrono::steady_clock::now() < deadline) Require(sceUsbdHandleEventsLocked(&longPass) == 0, "the handler failed while another thread waited"); + Require(finished.load(), "a thread that is not the handler did not return from sceUsbdHandleEventsLocked during a pass of event handling"); + follower.join(); + Require(waiterResult.load() == timedOut, "a thread that is not the handler did not time out in sceUsbdHandleEventsLocked during a longer pass of event handling"); + sceUsbdUnlockEvents(); + Require(sceUsbdEventHandlerActive() == 0, "the handler stayed active at the end"); +} + +void FollowerReturnsAfterHeldLock() { + UsbdTimeval brief{0, 20000}; + sceUsbdLockEvents(); + std::atomic entered{false}; + std::atomic finished{false}; + std::thread follower([&] { + sceUsbdLockEventWaiters(); + entered.store(true); + UsbdTimeval shortWait{0, 30000}; + sceUsbdHandleEventsLocked(&shortWait); + sceUsbdUnlockEventWaiters(); + finished.store(true); + }); while (!entered.load()) Require(sceUsbdHandleEventsLocked(&brief) == 0, "the handler failed while another thread queued"); - if (holdPastTimeout) std::this_thread::sleep_for(std::chrono::milliseconds(100)); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); while (!finished.load() && std::chrono::steady_clock::now() < deadline) Require(sceUsbdHandleEventsLocked(&brief) == 0, "the handler failed while another thread waited"); - Require(finished.load(), "a thread that is not the handler did not return from sceUsbdHandleEventsLocked while the handler released the event lock"); + Require(finished.load(), "a thread that is not the handler did not return from sceUsbdHandleEventsLocked after the handler held the event lock past its timeout"); follower.join(); - Require(waiterResult.load() == timedOut, "a thread that is not the handler did not time out in sceUsbdHandleEventsLocked"); sceUsbdUnlockEvents(); Require(sceUsbdEventHandlerActive() == 0, "the handler stayed active at the end"); } @@ -125,8 +168,9 @@ void EventLocks() { Require(woken.load() == 0 && handling.load() == 1, "a waiter was not woken and handed the events when the handler unlocked"); Require(sceUsbdEventHandlerActive() == 0, "the handler stayed active after the woken waiter unlocked"); - FollowerTimesOut(false); - FollowerTimesOut(true); + FollowerWakesAfterPass(); + FollowerTimesOutDuringPass(); + FollowerReturnsAfterHeldLock(); } void Pump(const int& done) { diff --git a/docs/dev/debt/libSceUsbd.md b/docs/dev/debt/libSceUsbd.md index a8c36d7991..2d81cf7b53 100644 --- a/docs/dev/debt/libSceUsbd.md +++ b/docs/dev/debt/libSceUsbd.md @@ -11,4 +11,4 @@ - [sceUsbdGetConfiguration, sceUsbdGetConfigDescriptorByValue, sceUsbdGetDescriptor, sceUsbdGetStringDescriptorAscii](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 ([libusb10_desc.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_desc.c), [libusb20.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb20.c)). The configuration and descriptor queries return host libusb errors. `sceUsbdGetStringDescriptorAscii` rebuilds `libusb20_dev_req_string_simple_sync` with only its 255-byte read: libusb's Windows HID backend reports the requested length as `bLength`, cutting a 4-byte header read to one character. Unlike the host `libusb_get_string_descriptor_ascii` it accepts index 0, drops non-printable characters and `<>` instead of writing `?`, and fails with `0x802400FF`. Closes when: measured on a PS5 - [sceUsbdGetDevice, sceUsbdGetDeviceSpeed, sceUsbdGetMaxPacketSize, sceUsbdGetMaxIsoPacketSize, sceUsbdOpenDeviceWithVidPid](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 [libusb10.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10.c) over the host libusb. The packet sizes are searched in the active configuration and the iso size is scaled as FreeBSD does, not through the host's versions, which differ for an unreadable configuration and for SuperSpeed endpoints. A host SuperSpeed+ device reports SuperSpeed, the fastest speed FreeBSD 11.4 has. A null device in `sceUsbdGetMaxIsoPacketSize` throws, as FreeBSD dereferences it. A device stays known while a handle to it is open, so the device of a handle from `sceUsbdOpenDeviceWithVidPid` can be used until that handle is closed. Closes when: measured on a PS5 - [sceUsbdSetInterfaceAltSetting, sceUsbdClearHalt, sceUsbdDetachKernelDriver](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 [libusb10.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10.c) over the host libusb. `sceUsbdSetInterfaceAltSetting` checks the interface range and this library's record of claimed interfaces, and returns `0x802400FF` for any other failure; it throws while the device has transfers in flight, which FreeBSD cancels first. `sceUsbdClearHalt` returns 0, or `0x802400FF` for an endpoint the host does not find; FreeBSD ignores the result of the clear itself, so the host's other errors throw, and so does a null handle, which FreeBSD dereferences. `sceUsbdDetachKernelDriver` returns `0x802400FF` for any failure, which on Windows is every call, as libusb cannot detach drivers there. Closes when: measured on a PS5 -- [sceUsbdLockEvents, sceUsbdTryLockEvents, sceUsbdUnlockEvents, sceUsbdLockEventWaiters, sceUsbdUnlockEventWaiters, sceUsbdWaitForEvent, sceUsbdEventHandlingOk, sceUsbdEventHandlerActive, sceUsbdHandleEvents, sceUsbdHandleEventsLocked, sceUsbdHandleEventsTimeout](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 [libusb10_io.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_io.c): one lock for events and waiters and one handler thread, reported by `sceUsbdEventHandlingOk`. Waiters wake when the handler unlocks and take over when there is none. The lock is released while the host libusb handles events. Taking the lock twice in one thread, releasing it or waiting without it, and a negative or oversized timeout throw. With no timeout, event handling returns once a transfer completes; FreeBSD also returns on its other poll events. Closes when: measured on a PS5 +- [sceUsbdLockEvents, sceUsbdTryLockEvents, sceUsbdUnlockEvents, sceUsbdLockEventWaiters, sceUsbdUnlockEventWaiters, sceUsbdWaitForEvent, sceUsbdEventHandlingOk, sceUsbdEventHandlerActive, sceUsbdHandleEvents, sceUsbdHandleEventsLocked, sceUsbdHandleEventsTimeout](../../../core/libs/prx/libSceUsbd/Export.cpp) (libSceUsbd) - not measured on a console; they follow FreeBSD 11.4 [libusb10_io.c](https://github.com/freebsd/freebsd-src/blob/d106002967eff712894ab8b5337bbbff106ce5e0/lib/libusb/libusb10_io.c): one lock for events and waiters and one handler thread, reported by `sceUsbdEventHandlingOk`. Waiters wake after each handling pass and when the handler unlocks, and take over when there is none. The host libusb handles events without the lock. Taking the lock twice in one thread, releasing or waiting without it, and a negative or oversized timeout throw. With no timeout, handling returns once a transfer completes; FreeBSD also returns on other poll events. Closes when: measured on a PS5