fix(device): release every claimed interface and stop the status transfer on close - #7
Merged
Merged
Conversation
…sfer on close
uvc_close() left a camera's kernel driver detached, so its /dev/videoN never
came back and only a USB unbind/bind recovered it. Reproduced on RK3588 with a
DJI Osmo Pocket 3; two independent causes.
The VideoControl status interrupt transfer re-arms itself from
_uvc_status_callback() and was never cancelled, so it kept firing while the
close released that same interface and reattached uvcvideo. The resubmission
reached usbfs on an unclaimed interface ('did not claim interface 0 before
use'), usbfs re-claimed it, and libusb_close() then dropped that claim without
rebinding anything. It is now cancelled and drained under a bounded wait before
any interface is touched, with status_mutex making the callback's
stopping-check and its resubmission atomic against the stop.
Separately, only the VideoControl interface was released, so a streaming
interface claimed by a negotiation that never reached uvc_stream_close() stayed
claimed until the usbfs fd closed - which rebinds nothing. Every claimed
interface is now released, VideoControl last, because reattaching the driver to
VideoControl is what makes uvcvideo probe the function and that probe claims the
streaming interfaces itself.
Four hardware-independent CTest cases wrap the libusb entry points into an
ordered operation log and drive the real uvc_close(). On the pre-fix code they
print the defect directly: submit, release_if(0), submit, attach_driver(0),
submit, close.
andrescera
added a commit
that referenced
this pull request
Jul 28, 2026
…cancel's NOT_FOUND (#8) PR #7's teardown fix is correct in shape but was not actually synchronized. Two defects, both in the uvc_close() path it added. status_xfer_submitted is written by _uvc_status_callback() on the libusb event thread and read by uvc_stop_status_xfer() on the closing thread. status_mutex was added for exactly that, and its own comment claims both flags are read and written under it, but three accesses sat outside it: the callback's terminal-status clear, the closing thread's poll loop, and uvc_free_devh()'s check. volatile supplies neither atomicity nor a happens-before edge, so on the weakly-ordered aarch64 this fork ships on, the close can observe the flag clear while the callback's earlier stores are still invisible and free both the transfer and the handle out from under it. Every access now goes through status_mutex and both flags lose the misleading volatile. The bounded drain takes and drops the mutex per iteration rather than spanning its sleep, which would block the callback it waits for. The flag is also set before libusb_submit_transfer() rather than after, so a callback completing between the two statements can no longer have its clear overwritten with a stale 1. Separately, a cancel returning LIBUSB_ERROR_NOT_FOUND was treated as drained. libusb documents that code as "not in progress, already complete, or already cancelled", and in the last case the callback has not run yet - so the close released the interface, freed a transfer whose cancellation was still pending (undefined behaviour by libusb's own contract) and freed the devh that the pending callback dereferences. The callback is now the only thing that clears the flag, and the existing bounded wait plus quarantine is the only exit. No deadlock is introduced: the lock order is uniform and one-way, status_mutex first and libusb entry points under it, and libusb invokes callbacks from its event thread with no internal transfer lock held and documents cancellation as asynchronous, so nothing re-enters status_mutex from inside libusb. A new libuvc.race case drives the real uvc_close() against a real event thread over repeated iterations, and LIBUVC_SANITIZE plus a CI job fail on any ThreadSanitizer report. On the pre-fix code TSan reports data races in _uvc_status_callback() and uvc_free_devh(), a heap-use-after-free in _uvc_status_callback(), and a destroy-of-a-locked-mutex in uvc_free_devh(). Also closes a coverage gap: every teardown case used VideoControl at interface 0 with at most one VideoStreaming interface, which a loop that merely special-cased index 0 would satisfy. Two cases now drive a nonzero VideoControl index with three scattered VideoStreaming interfaces. No production change was needed for them.
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
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.
What
uvc_close()left a UVC camera's kernel driver detached, so its/dev/videoNnever came back — only a manual USBunbind/bindrecovered it. Two independent teardown defects:uvc_open_internal()submits it whenever the VideoControl interface carries a status endpoint (optional in UVC, present on most cameras), and_uvc_status_callback()re-arms it after every completion.uvc_close()never cancelled or freed it, so it kept re-arming while the close released that same interface and handed it back touvcvideo.uvc_get_stream_ctrl_format_size()claims the streaming interface before probing and returnsUVC_ERROR_INVALID_MODEwithout releasing it, anduvc_stream_open_ctrl()'s failure path frees the stream handle without releasing it either — so any failed negotiation orphaned that interface.Why
Both end with a USB interface bound to no driver at all.
For (1) the resubmission reaches usbfs on an interface the process no longer claims:
usbfs re-claims the interface on that path, evicting the driver that had just been reattached, and the following
libusb_close()drops that claim without rebinding anything.For (2) the interface stays claimed until
libusb_close()drops the usbfs fd — which rebinds nothing either. The kernel says so directly:uvcvideoprobes interface 0 while interface 1 is still held by usbfs and logsNo streaming interface found for terminal 3, registering no video node.Release order is load-bearing. Reattaching the driver to VideoControl is what makes
uvcvideoprobe the whole UVC function, and that probe claims the VideoStreaming interfaces itself — so VideoControl must be released last.A natural A/B on one board shows this is device-class behaviour, not one camera. Same element, same binary, same failure mode; the only difference is the optional status endpoint:
19f7:00802ca3:0023bNumEndpoints=0)ep_81, 16 msdid not claim interface 0uvcvideo/uvcvideouvcvideo/ NONE/dev/videoNrestoredCameras with no status endpoint never submitted the transfer, so their teardown is byte-identical — which is why this looked model-specific.
How
uvc_stop_status_xfer()cancels the transfer and waits, bounded (LIBUVC_STATUS_STOP_TIMEOUT_MS, default 500 ms), before any interface is touched. A newstatus_mutexmakes the callback's stopping-check and its resubmission one critical section with the stop — without it the callback can read the flag as clear, lose the CPU, and still submit after the release. A drain that times out quarantines the handle exactly as a wedged stream already does, rather than freeing memory libusb still references.uvc_release_claimed_ifs()releases every interface indevh->claimed, VideoControl last.uvc_free_devh()no longer frees a transfer libusb still owns.How to verify
Four new
libuvc.teardown.*cases--wrapthe libusb entry points into an ordered operation log and drive the realuvc_close(). On the pre-fix code they print the defect verbatim:no_status_endpoint_unchangedis the negative control and passes both before and after.On hardware (Rock 5B+, DJI Osmo Pocket 3, 5 consecutive open/close cycles):
did not claim interface 0NONE/NONEuvcvideo/uvcvideoevery cycle/dev/videoNrestoredRisks
LIBUVC_STATUS_STOP_TIMEOUT_MS(500 ms) touvc_close(), and only for a device whose event thread is wedged — a cancellation normally lands within one endpoint interval (8–32 ms). Both bounds are overridable at compile time.status_mutexis taken only by the status callback and the stop;libusb_cancel_transfer()is asynchronous and never re-enters the callback, so there is no lock inversion.Deployment note: never leave a backup of the old shared object in the library search path —
ldconfigscans everylib*.so*and links by SONAME, so alibuvc.so.0.0.7.baksitting next to it will capturelibuvc.so.0.