Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent file handles from leaking on failed open. #67

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

bpstark
Copy link
Contributor

@bpstark bpstark commented Apr 1, 2024

Since the webcam open function only returns the open fh on success, we need to close the FH ourselves in the event that the subsequent tests fail.

Testing:
iterate over all files in /dev/video* and attempt to open each one as a webcam
then log the details of said video device if successfully opened as a webcam.
This was used to find the initial leak of file handles, as a device which opend fine, but failed to be recognized as a valid webcam (supportsVideoCapture/ supportsVideoStreaming)the handle would be lost.

Since the webcam open function only returns the open fh on success, we need to close
the FH ourselves in the event that the subsequent tests fail.
webcam.go Outdated
@@ -34,15 +34,25 @@ type Control struct {
// Open a webcam with a given path
// Checks if device is a v4l2 device and if it is
// capable to stream video
func Open(path string) (*Webcam, error) {
func Open(path string) (w *Webcam, err error) {
w = nil
Copy link
Collaborator

@edaniels edaniels Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: how about var success bool and the defer checks !success and set it to true at the end of the function. might make this less error prone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with that as well. Changed have been made!

@bpstark bpstark requested a review from edaniels April 1, 2024 15:57
@edaniels edaniels merged commit d456f6c into blackjack:master Apr 1, 2024
@bpstark bpstark deleted the close_error_fh branch April 4, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants