Debugging through poetry drops subprocess

[getglad]

Environment data

• debugpy version: 1.5.1
• OS and version: masOS 12.2.1
• Python version (& distribution if applicable, e.g. Anaconda): Python 3.9.10
• Using VS Code or Visual Studio: VS Code

Actual behavior

I have a click application (https://click.palletsprojects.com/en/8.0.x/) that is configured to run as a poetry script. I have a launch.json config that can successfully launch the script, accept the arguments, and hit breakpoints that I set inside of the click functions.

The issue is that the debugger detaches after 6 seconds no matter if breakpoints are set, the code is allow to run uninterrupted, using different arguments to trigger a different function, etc.

The call stack is showing the CLI file and the functions I break at to be inside a subprocess, and I have subprocess set to true, but I am wondering if this is part of the issue.

I have reviewed this ticket and would expect this to work fairly similarly.

Expected behavior

The debugger would launch and allow stepping, etc, as usual.

Steps to reproduce:

1. Create a click file (below is using the example from their hello world)

# testcli.py

import click

@click.command()
@click.option('--count', default=1, help='Number of greetings.')
@click.option('--name', default='Your name')
def hello(count, name):
    """Simple program that greets NAME for a total of COUNT times."""
    for x in range(count):
        click.echo(f"Hello {name}!")

if __name__ == '__main__':
    hello()

2. Create a poetry project with a script pointed to a python file using click

# pyproject.toml

...
[tool.poetry.scripts]
hello = 'testcli:hello'
...

3. Add a launch.json config that can run poetry

{
  "version": "0.2.0",
  "configurations": [
    {
      "name": "Foobar CLI",
      "type": "python",
      "request": "launch",
      "program": "${env:HOME}/.poetry/bin/poetry",
      "args": ["run", "hello"],
      "console": "integratedTerminal",
      "justMyCode": false,
      "subProcess": true,
    }
  ]
}

[int19h]

The short story is that Poetry uses execvpe() on Linux and macOS to spawn the child process "in place" - i.e. it replaces the parent process. We do detect this call and start debugging the new process, but at the same time we are still expecting the debug server in the original process to be in place, and wait for DAP events from it on the socket. The 6-second timeout is when the socket is detected as closed, which we interpret as process having exited. And since it was the parent process that exited, we automatically kill the child process and end the session.

Fixing this would require two things. First, pydevd should treat execvpe as not just spawning a new process, but also terminating the original process, and report "exit" and disconnect accordingly before delegating to the real implementation. We also need to make sure that code for auto-killing child processes handles this case properly, treating the child as the new root. @fabioz, what do you think?

[fabioz]

Fixing this would require two things. First, pydevd should treat execvpe as not just spawning a new process, but also terminating the original process, and report "exit" and disconnect accordingly before delegating to the real implementation. We also need to make sure that code for auto-killing child processes handles this case properly, treating the child as the new root. @fabioz, what do you think?

@int19h well, if we just make part 2 (make sure that code for auto-killing child processes handles this case properly, treating the child as the new root), that'd already be enough, no?

Part 1 is probably more of a bonus to have an early exit message without the 6-second timeout, but if part 2 is done this shouldn't really be a problem in practice (right now I think that if we do that it'll just stop sooner without any delay, so, it wouldn't be good -- we'd probably need a special message saying that this is about to exit but another one will connect soon if we do want to implement that part, but I'm not sure it's worth it...).

[int19h]

Yes, you're right, it would treat the entire multiproc session as over if the parent process exits before the child gets a chance to connect. And synchronizing that across processes would be very tricky to do right. But it should still work if we just let the process time out.

Ironically, the timeout will also work as a workaround for #43 until it's fixed.

[fabioz]

The fix for this is probably on the connection management on the debugpy side.

The caveat is:

The problem is that debugpy doesn’t have the ability to distinguish the two cases – one where the root process exits normally (where we do want to kill child processes if they are still running), and one where the root process is effectively replaced by another process via execve. So then it would still need that custom message from pydevd.

I think a custom message should be relatively straightforward to send. The tricky part here is that since the process will be soon replaced we need to make sure that the message is read by debugpy prior to that -- we can do a request/response message which expects a confirmation so that pydevd sends it and waits for the acknowledgement before calling os.execvpe. Does that sound reasonable?

Maybe something as PydevdNotifyProcessReplaceRequest / PydevdNotifyProcessReplaceResponse?

[int19h]

That would also make pydevd impossible to use directly by DAP clients that wouldn't know about this custom message.

I think an event is sufficient, though (maybe even just a custom attribute in "exited"?). Even if the process exits immediately after sending it, so long as it's not buffered on pydevd side, the socket on debugpy side will have the packet, and won't report itself as closed until that data is read. So debugpy will see the message first before noticing disconnect, and has a chance to set the "new process is coming" flag.

[fabioz]

I think that the pydevd side doesn't really send an exited event right now (as it's hard for it to know when it exits and return the proper code since the user can at any time have an application crash, this is managed in the debugpy side).

Given that, I think a custom event to note that the process will be replaced seems reasonable -- maybe a PydevdNotifyProcessReplaceEvent?

[int19h]

Yup, in general it can't reliably report exiting from inside the process. But this is one codepath where it's known that the process is, in fact, about to exit for sure, so I think it would be appropriate to use it here. For a client that doesn't support multiproc and uses pydevd directly, it would also serve as an indication of graceful process termination. OTOH code doesn't really matter in this case, since the OS process doesn't really have an exit code; it could be -1 or something similar.

[fabioz]

Ok, how about an exited event with a custom:

"reason": "processReplaced"

then?

[int19h]

Sounds good!

[getglad]

@int19h @fabioz saw that #883 was merged. Does that mean this should work now or is there additional work to come?

[int19h]

There's still additional work to do - that particular change was adding the necessary infrastructure, but it still has to be wired up properly in debugpy. This issue remains open to track that work.

[jappi00]

Hello together,
does somebody have a quick fix for this?

[susanhuhu]

I see this issue is fixed. So is it included in latest version of vs code?

[int19h]

The fix is in debugpy 1.63, which shipped in vscode-python for some time now. Are you still seeing the issue?