fix(screencapture): USB discovery/activation robustness (infinite retry loop, fatal-on-close, handle leak, Linux --udid) - #164
Open
LWHikarik wants to merge 1 commit into
Conversation
…on USB close, fix handle leak and Linux --udid match Four robustness fixes in the USB discovery/activation path. Two of them compound into the same failure: a leaked device handle makes the deferred context close fail, and that close error used to call log.Fatalf. - EnableQTConfig: the retry counter and its cap sat after the failure `continue`, so a persistently failing ReOpen never incremented it and the loop spun forever at 500ms/iteration, holding the device. Count every attempt before the continue, and close the freshly opened context when giving up. - createContext: a USB context-close error called log.Fatalf, which exits the process and tears down a live stream. Downgraded to a warning. - mapToIosDevice: device handles were only closed on the happy path, so a transient SerialNumber()/Product() read error (common under USB contention) leaked them. Close all handles via defer on every return path. - FindIosDevice: macOS libusb NUL-pads 24-character serials to 40 bytes while Linux returns them bare. ValidateUdid emits the padded form, so the direct comparison never matched on Linux. Trim NUL padding on both sides.
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.
Four small, independent fixes in
screencapture/, found while running qvhagainst iPhones on macOS and Linux. No API or behaviour changes beyond making
failures non-fatal and bounded.
1.
EnableQTConfigcould loop forever holding the deviceThe retry counter
i++and thei > 10cap were placed after thecontinuethat handles a failed
ReOpen. A device that consistently fails to re-enumerateinto the QuickTime configuration therefore never incremented the counter, and
the loop spun indefinitely at 500ms per iteration, keeping the device occupied.
The counter and cap now run before the failure path. The freshly created context
is also closed when giving up, instead of being leaked.
2. A USB context-close error killed the whole process
createContext's cleanup calledlog.Fatalfwhenctx.Close()returned anerror, which calls
os.Exit— terminating the process and tearing down any livevideo stream. A close error is not worth killing the process over; it is now
logged as a warning.
3. Device handle leak in
mapToIosDeviceHandles were only closed on the success path. A transient
SerialNumber()orProduct()read error — which happens under USB contention — returned early andleaked every handle opened so far. Those leaked handles are what made the
deferred
ctx.Close()fail, which then triggered thelog.Fatalfin fix #2.All handles are now closed via
deferon every return path.4.
--udiddid not match on LinuxmacOS libusb NUL-pads 24-character serials (iPhone Xr/Xs and newer) to 40 bytes,
whereas Linux libusb returns them bare. Since
ValidateUdidemits the paddedform, the direct
==comparison inFindIosDevicenever matched on Linux, and--udidsilently failed to find the device. Both sides are now NUL-trimmedbefore comparison.
Verified with
go build ./...andgo vet ./screencapture/on macOS (arm64,libusb 1.0.29). The fixes are base-agnostic — the two touched files are
identical on
mainandexternalizeGST.