Child processes are not killed when stopping the debugger

[DonJayamanne]

@DonJayamanne commented on Tue Jun 19 2018

Debug a python program that launches child processes.
Stop debugging, and confirm the child processes are killed.
This works in the old debugger, confirm this works as expected on the experimental debugger as well.

---

@DonJayamanne commented on Tue Jun 19 2018

Flashing as a bug even though it hasn't been confirmed, hence the needs verification flag.
Either way, it needs to be verified as this need to be looked into.

---

@DonJayamanne commented on Tue Jun 19 2018

• Confirmed
• Debug following code:

import subprocess

p = subprocess.Popen(['python', '-c', 'import time; time.sleep(10000)'])
print(p.pid)
p = subprocess.Popen(['python', '-c', 'import time; time.sleep(10000)'])
print(p.pid)
p = subprocess.Popen(['python', '-c', 'import time; time.sleep(10000)'])
print(p.pid)
p.wait()

• Stop the debugger
• The above processes are not killed.
  Code from old debugger https://github.com/Microsoft/vscode-python/blob/master/src/client/debugger/PythonProcess.ts#L69

---

@DonJayamanne commented on Thu Sep 27 2018

Upstream bug #503

---

@hjalmarlucius commented on Mon Feb 11 2019

Hi, I still have this issue. It does not occur for every file I run and am not sure how to trigger it but if I do heavy debugging using Pytorch and Visdom as main addins, I eventually run out of memory.

---

@bersbersbers commented on Mon Mar 11 2019

@DonJayamanne #503 was fixed and closed in November, but I am still seeing this issue (on Windows).

Interestingly, the above-mentioned fix involves a couple of atexits, such as atexit.register(kill_subprocesses). I myself have noticed that a workaround for killing processes called by subprocess which I came up with, using atexit too, was completely ineffective in VS Code on Windows. In particular, what I have noticed was that atexit works fine when running Python code in a terminal and closing it with Ctrl-C; but apparently, this is not what VS Code (Python) does, as atexit is not triggered when stopping debugging. (I had found the code that kills the Python process, and I believe it involves taskkill - so I am not surprised atexit does not work. I have to take this back, that was in https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/contrib/debug/node/debugAdapter.ts)

So would one solution on Windows be to send a Ctrl-C to the Python process instead of killing it, so that at least a user's workaround to end processes might work?

[DonJayamanne]

I can replicate this issue with the latest version of the debugger.
Stopping debugger doesn't kill child processes.
FYI - I have not turned on sub process debugging.

When I enable sub process debugging that gives me other errors:
See here

[int19h]

Subprocesses are only automatically killed in multiproc debugging context. Doing it always would break any scenario where they are not supposed to die - e.g. an app backgrounding itself.

The last one is definitely a bug, but a different one. Can you file that separately, with ptvsd logs for the processes involved?

[DonJayamanne]

This issue was reported previously on PTVSD and it was closed, #503

[int19h]

Sorry, I meant the issue with broken pipe.

Killing subprocesses in multiproc only is by design currently, but we can revisit that. The current implementation is tied to multiproc, because it knows what to kill based on debug messages it receives from those subprocesses (and their children) - it'd have to be remade to reconstruct the process tree by itself, which is possible, but not in a platform independent way. We'd need to either vendor something like psutil (which we already use for this purpose in tests), or reimplement it ourselves on all supported platforms.

[DonJayamanne]

The current implementation is tied to multiproc, because it knows what to kill based on debug messages it receives from those subprocesses (and their children)

Yes this will need to be re-visited.
It used to work in the old debugger, and that's why this issue was filled originally in ptvsd. I guess along the way it got changed into working only with sub processes...

[karthiknadig]

Closing this, since the investigation is complete.