Repository navigation
[bug fix] driver_vive: fix timeout-never-resubmits and missing clear_halt-on-STALL bugs - #378
Open
rocketmark wants to merge 1 commit into
Open
rocketmark wants to merge 1 commit into
rocketmark wants to merge 1 commit into
Conversation
…ALL bugs
Two bugs in handle_transfer()'s USB transfer retry logic:
1. On LIBUSB_TRANSFER_TIMED_OUT, below the consecutive-timeout/RF-device
disconnect threshold, the function returned without resubmitting the
transfer. Once an endpoint times out even once, nothing resubmits it
again, so it silently stops producing further transfer completions.
2. LIBUSB_TRANSFER_STALL was retried without calling libusb_clear_halt()
first, so the retry was guaranteed to stall again.
Both confirmed with a new regression test (driver_vive_timeout_stall_test.c)
that drives the real, unmodified handle_transfer/AttachInterface code
against a fake libusb (mock_libusb.c, no hardware needed).
Before this fix, with the included test:
consecutive_timeouts: 1, submit calls: 0 (never resubmitted)
clear_halt calls: 0 (expected 1) (retry guaranteed to
stall again)
After: both pass with the expected values.
Note: collabora#372 fixes two related but distinct bugs in the same function
(error_count double-incremented, and falling through to disconnect after
a successful resubmit instead of returning) -- this PR deliberately
leaves those two lines untouched to avoid overlapping with that PR.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
handle_transfer()'s USB transfer retry logic has two bugs: the timeout path never resubmits below the disconnect threshold, and a STALL is retried without clearing the halt condition first.
(Note: #372, open, fixes two other bugs in this same function — error_count double-incremented, and falling through to disconnect after a successful resubmit instead of returning. This PR deliberately leaves those two lines untouched to avoid overlapping with that PR.)
Demonstration
BEFORE fix:
status = LIBUSB_TRANSFER_TIMED_OUT, consecutive_timeouts = 1 (below the disconnect threshold)
libusb_submit_transfer() never called
Result: endpoint stops producing further completions, permanently
status = LIBUSB_TRANSFER_STALL
libusb_clear_halt() never called
(resubmit proceeds anyway)
Result: the retry is guaranteed to stall again
AFTER fix:
status = LIBUSB_TRANSFER_TIMED_OUT, consecutive_timeouts = 1
libusb_submit_transfer() called
Result: endpoint resubmitted, keeps operating
status = LIBUSB_TRANSFER_STALL
libusb_clear_halt() called
Result: halt condition cleared before the retry is submitted
Impact
Reachable any time a USB transfer doesn't complete cleanly — an ordinary timeout or stall, not an edge case. As it stands, any endpoint that times out even once stops producing further completions permanently, and a stalled endpoint's retry is guaranteed to fail the same way.
Change
One file, src/driver_vive.libusb.h. In handle_transfer(): (1) resubmit the transfer before returning on a sub-threshold timeout, (2) call libusb_clear_halt() before retrying a STALL. Includes a regression test (driver_vive_timeout_stall_test.c) against a fake libusb (mock_libusb.c), reproducing each bug independently and without hardware.
Found via
Found while auditing handle_transfer()'s retry/error-handling logic. Confirmed each bug independently with a deterministic test harness.