google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.59k stars 201 forks source link

Handle leak (not our fault) in EnergyLib64 #55

Open ariccio opened 9 years ago

ariccio commented 9 years ago

I don't think this is UIforETW's fault, but I can't figure out how to contact the Intel Power Gadget team (yet), and need to track it somewhere.

It appears that a HANDLE is leaked inside the power gadget library:

=======================================
VERIFIER STOP 0000000000000901: pid 0x4D38: A HANDLE was leaked. 

    0000000000000808 : Value of the leaked handle. Run !htrace <handle> to get additional information about the handle if handle tracing is enabled.
    0000000D6AC92360 : Address to the allocation stack trace. Run dps <address> to view the allocation stack.
    0000000D732BEFD0 : Address of the owner dll name. Run du <address> to read the dll name.
    00007FFA2AFA0000 : Base of the owner dll. Run .reload <dll_name> = <address> to reload the owner dll. Use 'lm' to get more information about the loaded and unloaded modules.

=======================================
This verifier stop is continuable.
After debugging it use `go' to continue.

=======================================

(4d38.68d4): Break instruction exception - code 80000003 (first chance)
vrfcore!VerifierStopMessageEx+0x6d0:
00007ffa`026d2190 cc              int     3
*** WARNING: Unable to verify checksum for UIforETW_devrel.exe
0:000> !htrace 0000000000000808 
--------------------------------------
Handle = 0x0000000000000808 - OPEN
Thread ID = 0x00000000000068d4, Process ID = 0x0000000000004d38

0x00007ffa429b3a4a: ntdll!NtCreateFile+0x000000000000000a
0x00007ffa02673e6f: vfbasics!AVrfpNtCreateFile+0x00000000000000ff
0x00007ffa3fd21e18: KERNELBASE!CreateFileInternal+0x0000000000000308
0x00007ffa3fd21af6: KERNELBASE!CreateFileW+0x0000000000000066
0x00007ffa0267438e: vfbasics!AVrfpCreateFileWCommon+0x0000000000000166
0x00007ffa0267449a: vfbasics!AVrfpKernelbaseCreateFileW+0x000000000000004a
0x00007ffa0267438e: vfbasics!AVrfpCreateFileWCommon+0x0000000000000166
0x00007ffa02674438: vfbasics!AVrfpKernel32CreateFileW+0x0000000000000058
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files\Intel\Power Gadget 3.0\EnergyLib64.dll - 
0x00007ffa2afa4201: EnergyLib64!IntelEnergyLibInitialize+0x0000000000000081
0x00007ff7b19c2d6b: UIforETW_devrel!CPowerStatusMonitor::CPowerStatusMonitor+0x00000000000001ab
0x00007ff7b19cd220: UIforETW_devrel!CUIforETWDlg::CUIforETWDlg+0x00000000000002a0
0x00007ff7b19cc34a: UIforETW_devrel!CUIforETWApp::InitInstance+0x00000000000000ca
0x00007ffa09d500ae: mfc120u!AfxWinMain+0x0000000000000076

--------------------------------------
Parsed 0x997 stack traces.
Dumped 0x1 stack traces.
0:000> dps 0000000D6AC92360 
0000000d`6ac92360  0000000d`6ac50470
0000000d`6ac92368  00090000`00007801
0000000d`6ac92370  00007ffa`02674438 vfbasics!AVrfpKernel32CreateFileW+0x58
0000000d`6ac92378  00007ffa`2afa4201 EnergyLib64!IntelEnergyLibInitialize+0x81
0000000d`6ac92380  00007ff7`b19c2d6b UIforETW_devrel!CPowerStatusMonitor::CPowerStatusMonitor+0x1ab [c:\users\alexander riccio\documents\github\uiforetw\uiforetw\powerstatus.cpp @ 383]
0000000d`6ac92388  00007ff7`b19cd220 UIforETW_devrel!CUIforETWDlg::CUIforETWDlg+0x2a0 [c:\users\alexander riccio\documents\github\uiforetw\uiforetw\uiforetwdlg.cpp @ 87]
0000000d`6ac92390  00007ff7`b19cc34a UIforETW_devrel!CUIforETWApp::InitInstance+0xca [c:\users\alexander riccio\documents\github\uiforetw\uiforetw\uiforetw.cpp @ 212]
0000000d`6ac92398  00007ffa`09d500ae mfc120u!AfxWinMain+0x76 [f:\dd\vctools\vc7libs\ship\atlmfc\src\mfc\winmain.cpp @ 37]
0000000d`6ac923a0  00007ff7`b19da57a UIforETW_devrel!__tmainCRTStartup+0x162 [f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c @ 618]
0000000d`6ac923a8  00007ffa`42142d92 KERNEL32!BaseThreadInitThunk+0x22
0000000d`6ac923b0  00007ffa`42929f64 ntdll!RtlUserThreadStart+0x34
0000000d`6ac923b8  00007ffa`09d500ae mfc120u!AfxWinMain+0x76 [f:\dd\vctools\vc7libs\ship\atlmfc\src\mfc\winmain.cpp @ 37]
0000000d`6ac923c0  00007ff7`b19da57a UIforETW_devrel!__tmainCRTStartup+0x162 [f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c @ 618]
0000000d`6ac923c8  00007ffa`42142d92 KERNEL32!BaseThreadInitThunk+0x22
0000000d`6ac923d0  00007ffa`42929f64 ntdll!RtlUserThreadStart+0x34
0000000d`6ac923d8  00007ffa`42142d92 KERNEL32!BaseThreadInitThunk+0x22

Breaking at the creation point shows what the handle actually is for:

Breakpoint 1 hit
KERNELBASE!CreateFileW:
00007ffa`3fd21a90 4883ec58        sub     rsp,58h
0:000> r
rax=0000000000000000 rbx=0000000000000003 rcx=00007ffa2ca59880
rdx=00000000c0000000 rsi=0000000000000000 rdi=00000000c0000000
rip=00007ffa3fd21a90 rsp=000000011345de78 rbp=00007ffa2ca59880
 r8=0000000000000000  r9=0000000000000000 r10=00007ffa0268c570
r11=000000011345e058 r12=0000000000000000 r13=0000000000000000
r14=00007ffa3fd21a90 r15=0000000000000000
iopl=0         nv up ei pl nz na pe nc
cs=0033  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000202
KERNELBASE!CreateFileW:
00007ffa`3fd21a90 4883ec58        sub     rsp,58h
0:000> du 00007ffa2ca59880
00007ffa`2ca59880  "\\.\EnergyDriver"

I found this while doing some unrelated refactoring, and, well... grrr.

randomascii commented 9 years ago

I have contacts at Intel so I can pass it along. Does it happen even if the DLL is unloaded? I take it there is no uninitialize function?

Avoiding resource leaks at shutdown is a hassle and sometimes not worth the trouble, but I can report it.

ariccio commented 9 years ago

Does it happen even if the DLL is unloaded?

Eh, you mean not loaded? No, I don't think so.

I take it there is no uninitialize function?

Nope, no documented function, and no exported function.

Reporting would be nice :)

randomascii commented 9 years ago

I meant does it happen even if the DLL is loaded and then unloaded.

If you want the Intel developers to investigate you probably need to create a minimal repro (preferably a single source file or a very small project) and precise instructions on how to repro the problem. For instance, specifying where to get App verifier, how to use it, and having something simpler than all of UIforETW to reproduce the problem.

ariccio commented 9 years ago

I meant does it happen even if the DLL is loaded and then unloaded.

No, if I do a LoadLibrary followed only by a FreeLibrary, then there's no leak.

If you want the Intel developers to investigate you probably need to create a minimal repro (preferably a single source file or a very small project) and precise instructions on how to repro the problem. For instance, specifying where to get App verifier, how to use it, and having something simpler than all of UIforETW to reproduce the problem.

Good idea, seems obvious in retrospect (duh on my part!). I've got a really simple minimal repro already thrown together. Shall I simply paste it into a comment?

randomascii commented 9 years ago

Pasting into a comment could work, but might be unwieldy by the time you have repro instructions and the necessary build commands. A .zip file with instructions and .vcxproj/.sln files would be one good option, or a github repo to allow tweaking it (I can test the instructions to make sure they work and provide feedback).

Be aware that a handle leak may be by design or at least not treated as high priority. Perfect cleanliness during shutdown is often pointless or not practical (see for example crbug.com/490716 which explicitly recommends calling TerminateProcess rather than paying the cost for a theoretically ideal but sometimes expensive shutdown).

ariccio commented 9 years ago

Be aware that a handle leak may be by design or at least not treated as high priority. Perfect cleanliness during shutdown is often pointless or not practical (see for example crbug.com/490716 which explicitly recommends calling TerminateProcess rather than paying the cost for a theoretically ideal but sometimes expensive shutdown).

Oh I totally agree, for application-level code, that is. The Intel Power Gadget API is third party code, and this is nowhere documented. If a program loads the library, initializes it, and frees it, then it's dealing with a mystery leak. It's quite easy to make my repro trigger this problem. Here it is leaking 6,730 handles:

energylibleak

6,730, next to Normal is Task Manager's Handles column.

...and all it took was:

    const size_t maxEnergyLib = 10000u;
    for ( size_t i = 0u; i < maxEnergyLib; ++i ) {
        doEnergyLib( );
        }

By leaking thousands of handles, we don't need Application Verifier to demonstrate the presence of a leak, but using Application Verifier will (of course) point to the offending HANDLE's allocation stack.

ariccio commented 9 years ago

Here's a repo: https://github.com/ariccio/EnergyLibLeak

randomascii commented 9 years ago

Thanks. I've passed it along. Good work.