All Terminal methods leak memory on Windows

[draivin]

All WinConsole methods start by calling conout, which in turn calls kernel32::CreateFileA, this creates a new file handle that is not destroyed.

I've solved this locally by wrapping the handle in a struct that calls kernel32::CloseHandle on drop.

UMDH log:

+    4030 (  4030 -     0)      2 allocs    BackTrace333F5AF9
+       2 (     2 -     0)  BackTrace333F5AF9   allocations

    ntdll!LdrOpenImageFileOptionsKey+53
    ntdll!RtlAllocateHeap+1182
    KERNELBASE!CreateFileA+5D
    walk!term::win::conout+8D (...\contrib\term\src\win.rs, 79)
    walk!term::win::WinConsole<std::io::stdio::Stdout>::apply<std::io::stdio::Stdout>+47 (...\contrib\term\src\win.rs, 102)
    walk!term::win::{{impl}}::reset<std::io::stdio::Stdout>+5F (...\contrib\term\src\win.rs, 193)
    ...

+    2018 (  2018 -     0)      1 allocs    BackTrace333F5F39
+       1 (     1 -     0)  BackTrace333F5F39   allocations

    ntdll!LdrOpenImageFileOptionsKey+53
    ntdll!RtlAllocateHeap+1182
    KERNELBASE!CreateFileA+5D
    walk!term::win::conout+8D (...\contrib\term\src\win.rs, 79)
    walk!term::win::WinConsole<std::io::stdio::Stdout>::apply<std::io::stdio::Stdout>+47 (...\contrib\term\src\win.rs, 102)
    walk!term::win::{{impl}}::bg<std::io::stdio::Stdout>+5F (...\contrib\term\src\win.rs, 160)
    ...

Tagging @retep998.

[Stebalien]

If @retep998 doesn't beat me to it, I'll get around to fixing this after labor day (thesis deadline).

[TheNain38]

Is there any news about this?

[Stebalien]

Other than it completely fell off my radar? No, sorry. I'll try to fix it tomorrow. If I don't fix it in a few days, please bug me.

[Ralith]

Any updates on this? I'm seeing some weird behavior on windows, and it'd be nice to eliminate this as the cause.

[Ralith]

bugging intensifies

edit: For the record, the weird behavior turned out to be caused by unrelated buffering errors, but this amount of leakage still makes me uncomfortable.

[Stebalien]

I'm unlikely to get around to fixing this any time soon as I don't have access to a windows machine and debugging with wine is a real pain. If anyone has a Windows machine, I'd be happy to review a patch.

[swolchok]

I'm not familiar with Windows debugging tools, so I had a really "fun" time reproing this on Windows 10. I wasn't able to get UMDH to work, so I followed https://blogs.technet.microsoft.com/yongrhee/2011/12/19/how-to-troubleshoot-a-handle-leak/ .

Condensed steps I took, not all of which I'm sure are necessary: 1) Install Debugging Tools for Windows following https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/ . 2) Checkout and build https://github.com/facebookincubator/fastmod, which uses term for colored output. 3) Launch Application Verifier (installed via Debugging Tools for Windows). Enable "Basics" for fastmod.exe by pressing Ctrl-A, navigating to fastmod\target\debug\fastmod.exe, and then making sure "Basics" is checked in the right pane of Application Verifier. 4) From fastmod's root directory, run target\debug\fastmod.exe fn fun. It's important to use cmd.exe and not bash or something, because otherwise a TerminfoTerminal will get created instead of the Windows-specific one. 5) Start windbg (cmd-r windbg) and use File > Attach to Process to attach to fastmod.exe. 6) In windbg, run !htrace -snapshot and then g. 7) In fastmod, press n to reject the first change. 8) In windbg, select Break from the Debug Menu and then run !htrace -diff. Note that there are about 2 outstanding handles since the last snapshot, indicating a leak. Run g to resume fastmod. 9) Repeat steps 7 and 8 a few times and note that the number of outstanding handles grows.

I have a local patch that creates a wrapper struct that calls CloseHandle and stops the number of outstanding handles from growing, as suggested in the original issue. I hope to send a PR sometime in the near future. The irritating thing about patching this is that auto-dereferencing for the wrapper I wrote didn't quite work (perhaps because HANDLE is a raw pointer?), so I had to write *handle instead of handle everywhere.