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

Windows: Fixes for corner case leaks when reusing device references #64

Closed
wants to merge 3 commits into from

Conversation

projectgus
Copy link
Contributor

I noticed my libusb-based program was leaking memory under Windows and tracked it down to leaks that occur when libusb_get_device_list() reuses existing device references.

A test program that demonstrates the problem is here:
https://gist.github.com/projectgus/c9e29f90397202dbba38

  • When run on Windows with current libusb the debug output includes "some libusb_devices were leaked" near the end, even though all instances are released. (related to 0cbb891)
  • When run under drmemory leaks are reported on close. (related to f6b348f)
  • If run with the RUN_INFINITE variable changed to 1 then the memory usage will climb indefinitely.

The three commits in this PR fix these problems for me.

I've been cross-compiling with mingw and testing on Windows 7 and XP 32-bit. But I don't think those details are relevant.

BTW, thanks very much to everyone who works on libusb. It's marvellous being able to write cross-platform code for something as complex as USB, and have it run everywhere. I am even more admiring of libusb after today, having now become more familiar with the actual Windows USB stack. ;)

If windows_get_device_list() found a device that already existed, it
would erronenously increment the parent_dev refcount and cause a
reference leak - the parent device would never be released.
windows_get_device_list was leaking dev_interface_path (in either of two
possible places) when a pre-existing device reference was reused.
As per comment in existing code, get_devinfo_data() should be called
repeatedly until it returns false indicating the handle has been
released. When testing I could see cases where this didn't happen.
However it didn't seem like my program actually leaked any handles. This
commit errs on the safe side anyhow.
@dickens
Copy link
Member

dickens commented Mar 2, 2016

Applied the first two commits. The third is not needed, so it was omitted. Thanks for submitting this!

@dickens dickens closed this Mar 2, 2016
@rezahousseini
Copy link

rezahousseini commented Jul 2, 2020

Where is this pull request merged? I still have this issue with version 1.0.23

@mcuee mcuee added the windows label Jul 4, 2021
@mcuee
Copy link
Member

mcuee commented Jul 4, 2021

@rezahousseini Please create a new issue if you still see the issue in 1.0.24 or latest git. Thanks.
The codes have been totally changed.
https://github.com/libusb/libusb/blob/master/libusb/os/windows_winusb.c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants