winpty leaks processes

[Tyriar]

Forked from https://github.com/Microsoft/vscode/issues/26807, node servers in particular don't get killed when killing the winpty agent. Grabbing the process list via GetConsoleProcessList and killing them manually in winpty is probably the right solution.

[rprichard]

Thanks for filing the issue. I don't know when I'll get around to working on this issue, but it's probably something that should be fixed (or at least improved).

Comments:

• I occasionally think jobs are the fix for this issue, but I suspect they don't help because a GUI program spawned from a winpty shell would be part of the same job as the shell (probably?), and then closing the winpty terminal would terminate the GUI program.

• Ordinarily, closing a console sends the processes a CTRL_CLOSE_EVENT signal, giving them a chance to clean up (for ~5 seconds). It would be good if winpty maintained this behavior rather than simply terminate all processes on the console process list.

• If winpty were to terminate a PID from the console process list, then perhaps it should double-check that the PID is still on the console process list after opening the process handle. Otherwise, there is a race condition where a PID could be reused after calling GetConsoleProcessList but before calling OpenProcess.

• It looks like GenerateConsoleCtrlEvent is unable to synthesize CTRL_CLOSE_EVENT. I tested a GenerateConsoleCtrlEvent(CTRL_CLOSE_EVENT, 0) call on Win10_16257, and MSDN states that only CTRL_C_EVENT and CTRL_BREAK_EVENT are allowed.

• The GetConsoleProcessList process order is wrong in 15063's v2 console (https://github.com/Microsoft/BashOnWindows/issues/2170).

• The process order also determines the order that the console tries to kill processes. I believe (but haven't conclusively determined) that the process order is reverse of (or the same as) the order that processes attached to the console. (XXX: If a process detaches and reattaches, its PID should move to the front of (or the rear of) the process list. This should be tested.)

• I really need to test what happens if processes detach or attach from a console during the close operation. I already determined that if a process in the console list exits prematurely during the close operation, the close operation aborts. (See closetest.cc in https://github.com/Microsoft/BashOnWindows/issues/2170.)

Possible solution:

winpty can determine whether the console process order is in correct order or reversed order by looking for its own PID in the process list.

• If the order is correct, the agent process could close the console by repeatedly sending/posting WM_CLOSE until the agent process terminates. I'm guessing posting won't work because it won't block, and even a successful close operation operation can take arbitrarily long. (See closetest.cc in https://github.com/Microsoft/BashOnWindows/issues/2170 for an example.) Does SendMessage of WM_CLOSE block for the correct amount of time, and could it deadlock?

• If the order is wrong, the agent process could spawn a process whose job was to clean up. The cleanup task would repeatedly send/post WM_CLOSE, and it would be the last process to be killed, unless other/new processes attach to the console. Maybe it could have a CTRL_CLOSE_EVENT handler that respawns itself if it sees that it isn't the last process remaining in the console process list.

[Sader82]

Is there a solution already? I'm running the latest version of vscode and powershell module, and still have the leaking process issue

[Tyriar]

@Sader82 the problem is fixed in the latest insiders by making use of https://github.com/rprichard/winpty/pull/130

I'm not sure what you're seeing but it's different to the leaking processes problem, where processes would be orphaned (no longer descendants of the Code.exe tree) and continue running.

[Sader82]

@Tyriar this happends when the integrated terminal for powershell is active. killing the terminal stops cloning (leaking) the process.

Perhaps it is fixed in the new version. I do not run ths insider build..