tests: Add tests/testcore.c, a multi-threaded stress test that hammers - #1809
Conversation
3a6cca4 to
1d05add
Compare
libusb_get_device_list / libusb_free_device_list from N threads in parallel. It exists primarily to reproduce the concurrent-enumeration crashes reported in libusb#1793 and the related set_composite_interface race surfaced during PR libusb#1795 review. The stressed code paths include: - usbi_get_device_by_session_id() - usbi_alloc_device() - usbi_connect_device() - libusb_unref_device() / ctx->usb_devs_lock - winusb_device_priv setup races in set_composite_interface and set_hid_interface (Windows, non-hotplug builds) The original C++20 reproducer was posted by smarvonohr and the C version with Win32 threads by mcuee in libusb#1795 (comment) This version is adapted from mcuee's C version, with two changes that make it portable and easier to build: - Use the same PLATFORM_POSIX / PLATFORM_WINDOWS thread abstraction that stress_mt.c uses (pthread_create on POSIX, _beginthreadex on Windows, CreateThread on Cygwin), instead of bare pthread. The original would not build with MSVC. - Print the device list once before starting the worker threads, so the operator can confirm at a glance which devices are present. This is useful because the bug only manifests when enumeration touches certain device shapes (e.g. composite devices with HID children that exercise set_composite_interface). Add msvc/testcore.vcxproj following the same template as msvc/stress_mt.vcxproj so the test builds out of the box in Visual Studio against libusb_static. Notes for runners: - The test only triggers the enumeration races in non-HOTPLUG builds. With Windows hotplug enabled, libusb_get_device_list is served from a cache and winusb_get_device_list never runs concurrently, so the bug does not manifest. - The default LOOPS = 100000 is intentionally large for the original bug-hunting use case but takes hours of real work even when the library is correct (each non-hotplug get_device_list does a full Windows USB enumeration, ~10-100 ms). Reduce LOOPS for a quick smoke test. - At least one composite USB device with HID-class child interfaces (USB game controller, headset with controls, Logitech Unifying receiver, etc.) must be present to exercise set_composite_interface. Pure UVC/UAC composite devices like webcams will NOT trigger that path because their child interfaces are not HID class and are not bound to any libusb-supported driver. Closes libusb#1808 Closes libusb#1809
|
Two quick comments.
|
|
For example, under macOS, without adding the new test to auto-tools build, it will not build testcore by default. Manual build: |
It would be good to make number of threads and number of loops to be arguments to the test program. If we choose the default loops to be 100,000, we may want to inform the user the above info. |
libusb_get_device_list / libusb_free_device_list from N threads in parallel. It exists primarily to reproduce the concurrent-enumeration crashes reported in libusb#1793 and the related set_composite_interface race surfaced during PR libusb#1795 review. The stressed code paths include: - usbi_get_device_by_session_id() - usbi_alloc_device() - usbi_connect_device() - libusb_unref_device() / ctx->usb_devs_lock - winusb_device_priv setup races in set_composite_interface and set_hid_interface (Windows, non-hotplug builds) The original C++20 reproducer was posted by smarvonohr and the C version with Win32 threads by mcuee in libusb#1795 (comment) This version is adapted from mcuee's C version, with two changes that make it portable and easier to build: - Use the same PLATFORM_POSIX / PLATFORM_WINDOWS thread abstraction that stress_mt.c uses (pthread_create on POSIX, _beginthreadex on Windows, CreateThread on Cygwin), instead of bare pthread. The original would not build with MSVC. - Print the device list once before starting the worker threads, so the operator can confirm at a glance which devices are present. This is useful because the bug only manifests when enumeration touches certain device shapes (e.g. composite devices with HID children that exercise set_composite_interface). Add msvc/testcore.vcxproj following the same template as msvc/stress_mt.vcxproj so the test builds out of the box in Visual Studio against libusb_static. Notes for runners: - The test only triggers the enumeration races in non-HOTPLUG builds. With Windows hotplug enabled, libusb_get_device_list is served from a cache and winusb_get_device_list never runs concurrently, so the bug does not manifest. - The default LOOPS = 100000 is intentionally large for the original bug-hunting use case but takes hours of real work even when the library is correct (each non-hotplug get_device_list does a full Windows USB enumeration, ~10-100 ms). Reduce LOOPS for a quick smoke test. - At least one composite USB device with HID-class child interfaces (USB game controller, headset with controls, Logitech Unifying receiver, etc.) must be present to exercise set_composite_interface. Pure UVC/UAC composite devices like webcams will NOT trigger that path because their child interfaces are not HID class and are not bound to any libusb-supported driver. Closes libusb#1808 Closes libusb#1809
1d05add to
7fc3b1c
Compare
|
Looks good to me with the latest changes. |
libusb_get_device_list / libusb_free_device_list from N threads in parallel. It exists primarily to reproduce the concurrent-enumeration crashes reported in libusb#1793 and the related set_composite_interface race surfaced during PR libusb#1795 review. The stressed code paths include: - usbi_get_device_by_session_id() - usbi_alloc_device() - usbi_connect_device() - libusb_unref_device() / ctx->usb_devs_lock - winusb_device_priv setup races in set_composite_interface and set_hid_interface (Windows, non-hotplug builds) The original C++20 reproducer was posted by smarvonohr and the C version with Win32 threads by mcuee in libusb#1795 (comment) This version is adapted from mcuee's C version, with two changes that make it portable and easier to build: - Use the same PLATFORM_POSIX / PLATFORM_WINDOWS thread abstraction that stress_mt.c uses (pthread_create on POSIX, _beginthreadex on Windows, CreateThread on Cygwin), instead of bare pthread. The original would not build with MSVC. - Print the device list once before starting the worker threads, so the operator can confirm at a glance which devices are present. This is useful because the bug only manifests when enumeration touches certain device shapes (e.g. composite devices with HID children that exercise set_composite_interface). Add msvc/testcore.vcxproj following the same template as msvc/stress_mt.vcxproj so the test builds out of the box in Visual Studio against libusb_static. Notes for runners: - The test only triggers the enumeration races in non-HOTPLUG builds. With Windows hotplug enabled, libusb_get_device_list is served from a cache and winusb_get_device_list never runs concurrently, so the bug does not manifest. - The default LOOPS = 100000 is intentionally large for the original bug-hunting use case but takes hours of real work even when the library is correct (each non-hotplug get_device_list does a full Windows USB enumeration, ~10-100 ms). Reduce LOOPS for a quick smoke test. - At least one composite USB device with HID-class child interfaces (USB game controller, headset with controls, Logitech Unifying receiver, etc.) must be present to exercise set_composite_interface. Pure UVC/UAC composite devices like webcams will NOT trigger that path because their child interfaces are not HID class and are not bound to any libusb-supported driver. Closes libusb#1808 Closes libusb#1809
- Make thread and loop counts command-line arguments with -h/--help usage text, defaulting to 4 threads and 10000 loops so the test stays fast under 'make check', and warn in the usage text that large loop counts can run for hours on non-hotplug backends - Replace the THREADS/LOOPS #defines with typed static const defaults and allocate the per-thread arrays dynamically - Add a commented-out LIBUSB_OPTION_NO_DEVICE_DISCOVERY hint for exercising the Linux non-hotplug code path - Build testcore via autotools, mirroring stress_mt including the Emscripten PROXY_TO_PTHREAD link flags - Add testcore.vcxproj to the Visual Studio solution - Pass the initialized context to the initial device listing instead of NULL, and propagate worker failures to the exit code Note: will be squashed before merging the PR
7fc3b1c to
77241e3
Compare
Would you be OK to approve the PR? I think merging it won't harm, no? |
|
Okay from my side. Approved. |
libusb_get_device_list / libusb_free_device_list from N threads in parallel. It exists primarily to reproduce the concurrent-enumeration crashes reported in #1793 and the related set_composite_interface race surfaced during PR #1795 review. The stressed code paths include: - usbi_get_device_by_session_id() - usbi_alloc_device() - usbi_connect_device() - libusb_unref_device() / ctx->usb_devs_lock - winusb_device_priv setup races in set_composite_interface and set_hid_interface (Windows, non-hotplug builds) The original C++20 reproducer was posted by smarvonohr and the C version with Win32 threads by mcuee in #1795 (comment) This version is adapted from mcuee's C version, with two changes that make it portable and easier to build: - Use the same PLATFORM_POSIX / PLATFORM_WINDOWS thread abstraction that stress_mt.c uses (pthread_create on POSIX, _beginthreadex on Windows, CreateThread on Cygwin), instead of bare pthread. The original would not build with MSVC. - Print the device list once before starting the worker threads, so the operator can confirm at a glance which devices are present. This is useful because the bug only manifests when enumeration touches certain device shapes (e.g. composite devices with HID children that exercise set_composite_interface). Add msvc/testcore.vcxproj following the same template as msvc/stress_mt.vcxproj so the test builds out of the box in Visual Studio against libusb_static. Notes for runners: - The test only triggers the enumeration races in non-HOTPLUG builds. With Windows hotplug enabled, libusb_get_device_list is served from a cache and winusb_get_device_list never runs concurrently, so the bug does not manifest. - The default LOOPS = 100000 is intentionally large for the original bug-hunting use case but takes hours of real work even when the library is correct (each non-hotplug get_device_list does a full Windows USB enumeration, ~10-100 ms). Reduce LOOPS for a quick smoke test. - At least one composite USB device with HID-class child interfaces (USB game controller, headset with controls, Logitech Unifying receiver, etc.) must be present to exercise set_composite_interface. Pure UVC/UAC composite devices like webcams will NOT trigger that path because their child interfaces are not HID class and are not bound to any libusb-supported driver. Closes #1808 Closes #1809
|
There is 1 downside to this development - it adds ~1.5-2 min to a single CI run. Maybe it is not much as an absolute value, but relatively that is 5m42s -> 9m23s (2 build runs) for one of the jobs, and 2m07 -> s3m01s for a different one, which is ~30% increase in CI run time. Need to consider what to do with it. Either leave it as is or try to optimize something, e.g. do not run these tests(s) on each PR, but leave it for master pushes, etc. |
|
I think we can keep this only for git commit to the master. |
|
I'm accustomed to CI taking hours, so that increase doesn't bother me. |
Keep testcore built while excluding it from default make check runs. Add --enable-testcore and enable it on master or via force_testcore/force_long_tests PR labels. Handle label-triggered workflow runs without cancelling normal CI. Related to #1809 Assisted-by: codex:gpt-5.6-sol Assisted-by: claude-code:claude-fable-5
libusb_get_device_list / libusb_free_device_list from N threads in parallel. It exists primarily to reproduce the concurrent-enumeration crashes reported in #1793 and the related set_composite_interface race surfaced during PR #1795 review.
The stressed code paths include:
Address issue: race in libusb get device list and libusb free device list with win usb backend + claim_interface race #1795 (comment) This version is adapted from mcuee's C version, with two changes that make it portable and easier to build:
Notes for runners:
Closes #1808