top | item 45532681

(no title)

brucedawson | 4 months ago

In 2021 I found an invisible memory leak in a tool (VS Code) that I have never used. This is the story of how.

discuss

order

canucker2016|4 months ago

If you look at the commits for the process.cc file, you can see that the ORIGINAL version of the GetProcessMemoryUsage function was correct.

for original function, see process.cc, lines 36-53 in https://github.com/microsoft/vscode-windows-process-tree/com...

A week later, when the GetCpuUsage function was added to process.cc, the CloseHandle call at the end of GetProcessMemoryUsage was deleted/pushed into GetCpuUsage.

for updated code, see process.cc, lines 53-96, in https://github.com/microsoft/vscode-windows-process-tree/com...

The unified diff for that change doesn't point out that code was deleted from GetProcessMemoryUsage (the CloseHandle() call is now in GetCpuUsage).

Someone ok'd those changes...

Also MS's PREfast/PREfix static code analysis tools missed that bug as well. I'd update my static analysis perl script to check for that case, but one of the comments to TFA said chatgpt will catch the problem, so I'll punt.

Minor nit: The ProcessInfo struct declares fields in an awkward fashion if you're debugging, esp. if you're viewing memory of the struct. One should put small size fields at the start of the struct, large size fields at the end. As is, the large name character array will occupy a large amount of space and if you want to see the values of the small fields, one would probably have to scroll to locate these small fields after the large character array.

Minor nit: What's with the magic number 1024? At least in recent versions of the process.cc file, they've gone from a fixed 1024 element array (each element == MAX_PATH * TCHAR + sizeof(DWORD)*3 bytes) to a dynamic array.