Skip to content

Unmanaged memory leak in WindowsHidApiService.GetHidCapabilities #219

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

Open
VerusMaya opened this issue Aug 27, 2021 · 3 comments
Open

Unmanaged memory leak in WindowsHidApiService.GetHidCapabilities #219

VerusMaya opened this issue Aug 27, 2021 · 3 comments

Comments

@VerusMaya
Copy link

Describe the issue
I observed an ongoing unmanaged memory leak when regularly polling for connected devices. I reproduced it with a minimal CLI .exe and narrowed it down to WindowsHidApiService.GetHidCapabilities. To resolve the leak, change the HidD_FreePreparsedData pointerToPreparsedData parameter to not be ref.

The HidD_GetPreparsedData documentation shows a *PreparsedData parameter. This matches with the out parameter option used.
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/hidsdi/nf-hidsdi-hidd_getpreparseddata

The HidD_FreePreparsedData documentation shows a PreparsedData parameter with no dereference. I believe it should therefore be called without ref.
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/hidsdi/nf-hidsdi-hidd_freepreparseddata

Relevant call stack -
WindowsHidApiService.GetHidCapabilities
WindowsHidApiService.GetDeviceDefinition
WindowsHidDeviceFactoryExtensions.GetDeviceDefinition
WindowsDeviceEnumerator.GetConnectedDeviceDefinitionsAsync

Screenshots
Memory profiler running for one minute. Notice the bottom-right section for unmanaged memory breakdown by module.

hid memory leak - 1m
without fix

hid memory leak - 1m fixed
with fix

Your Code
The memory leak can be reproduced easily.

static async Task Main(string[] args) {
	IDeviceFactory factory = new FilterDeviceDefinition(0x1234, 0x5678).CreateWindowsHidDeviceFactory();
	while (true) {
		_ = await factory.GetConnectedDeviceDefinitionsAsync();
		await Task.Delay(TimeSpan.FromMilliseconds(100));
	}
}

The leak can be resolved with two changes.

private static extern bool HidD_FreePreparsedData(ref IntPtr pointerToPreparsedData);

private static extern bool HidD_FreePreparsedData(IntPtr pointerToPreparsedData);

isSuccess = HidD_FreePreparsedData(ref pointerToPreParsedData);

isSuccess = HidD_FreePreparsedData(pointerToPreParsedData);

Log / Stack Trace
No relevant logging or stack trace.

Info

  • Platform: Windows 10
  • Device Type: HID
  • Version: 4.2.1
  • .NET: 5.0

PR coming soon.

@VerusMaya
Copy link
Author

Following up since it has been a bit.

Have you had a chance to look into this?

It would be great to get an updated nuget package with the memory leak resolved.

Thank you!

@Resonanz
Copy link

?

@HossamElwahsh
Copy link

Great work finding this. 💯 forked and fixed. until this gets merged into main branch.

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

No branches or pull requests

3 participants