Oshi: Handle leak in WindowsDisplay#getDisplays

Created on 27 Aug 2019  路  5Comments  路  Source: oshi/oshi

Hello,

As the title says there is handle leak in class WindowsDisplay method getDisplays()
https://github.com/oshi/oshi/blob/master/oshi-core/src/main/java/oshi/hardware/platform/windows/WindowsDisplay.java#L76

        WinNT.HANDLE hDevInfo = SetupApi.INSTANCE.SetupDiGetClassDevs(monitorGuid, null, null,
                SetupApi.DIGCF_PRESENT | SetupApi.DIGCF_DEVICEINTERFACE);

This handle hDevInfo is allocated and never closed

confirmed bug first-timers-only good first issue

All 5 comments

I'm marking this issue for first-timers-only. That means that I will only accept a PR for this one from someone who's never contributed to open source before. This one is easy (but don't make that statement make you feel bad if you have a hard time with it, there's more to contributing to open source than changing lines of code, especially if it's your first time). I'll hold your hand through this if you need me to. :-)

Per the report above, we allocate a handle with a call to SetupDiGetClassDevs. The docs state:

The caller of SetupDiGetClassDevs must delete the returned device information set when it is no longer needed by calling SetupDiDestroyDeviceInfoList.

This is easily accomplished with a call to JNA's implementation, e.g.,
SetupApi.INSTANCE.SetupDiDestroyDeviceInfoList(hDevInfo);

Optionally, to improve code readability, refactor the local variable monitorGuid to be a private static final GUID constant GUID_DEVINTERFACE_MONITOR.

Here are the steps to get a PR merged here.

  • Read the CONTRIBUTING file for reference. I'll walk you through anything you don't understand.
  • Fork the project to your own GitHub account, make a local clone, and create a branch for your submission.
  • Add the call to SetupDiDestroyDeviceInfoList() described above at the end of the if conditional on the successful handle return (after line 103).
  • Commit your changes, push to to your GitHub fork, and create a Pull Request
  • Respond to the usually minor comments on code review
  • Celebrate! :tada:

Hi. Can I take a stab at this? While I have been on github for quite sometime, I have never really contributed to open-source projects.

Go for it, @r10a!

@dbwiddis I think I patched the issue. Can you please let me know if I got it right?

Not quite. If the initial handle query is invalid, your value for hDevInfo is not a real handle and you shouldn't release it. So you need to move your change up one line.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Sami32 picture Sami32  路  8Comments

dbwiddis picture dbwiddis  路  12Comments

jflefebvre06 picture jflefebvre06  路  9Comments

andrea9293 picture andrea9293  路  8Comments

dbwiddis picture dbwiddis  路  12Comments