Support restart in terminated event in debug adapter

[nturinski]

It would be really handy if Python launch configurations had a similar property to Node.js's restart property. For reference: https://code.visualstudio.com/docs/nodejs/nodejs-debugging#_restarting-debug-sessions-automatically-when-source-is-edited.

The use case is whenever a python file is edited and saved, it causes the debugger to detach so it'd be nice to have an option to automatically re-attach the debugger in those instances.

[karthiknadig]

Thank you for the suggestion! We have marked this issue as "needs decision" to make sure we have a conversation about your idea. We plan to leave this feature request open for at least a month to see how many 👍 votes the opening comment gets to help us make our decision.

[stefanushinardi]

Hello, just to give a few data points/verbatim from other github threads(microsoft/vscode-azurefunctions#781 & Azure/azure-functions-host#3543) regarding customer pain points that this change might alleviate

Thanks!

[anirudhgarg]

Adding +1 on this - we have many customers in Azure Functions Python VSCode experience

[stefanushinardi]

Hello @karthiknadig, do you think there is already enough data points for this work item? Thanks!

[karthiknadig]

@stefanushinardi Thanks for the data points. When python files are edited, which process is doing the reload? is there a managing process that handles terminating and reloading the project. If yes, then is that process running on python?

/cc @fabioz @int19h

[stefanushinardi]

Pinging @nturinski and @Hazhzeng who are the experts on this

[nturinski]

When python files are edited, which process is doing the reload? is there a managing process that handles terminating and reloading the project. If yes, then is that process running on python?

I honestly rarely work with Python so I really don't know for sure, but I think VS Code is using this repo to do its debugging: https://github.com/microsoft/debugpy. So I imagine that there should be a way to watch the files via the debugpy process while its debugging and then have the Python extension reattach after a change.

[karthiknadig]

@nturinski The issue is we don't know which process it is if the restarted python process was not started by a parent that was already running under the debugger.

I am one of the authors of debugpy, and we support this for projects like django which handles re-load on save. They way it works there is, the main django process is run under the debugger. django then starts multiple worker processes which render the webpage, since these processes were started by a process with debugger already in it, the debugger monitors all the processes under the main process. When a developer makes changes to python code, django detects it terminates the sub-process that has that file loaded, and start an new sub-process with the updated code. They key here is that the django main process is not killed, so the debugger is still able to handle the newly spawned sub-processes. If there is a main root process that has the debugger running in it then this should work if the debugger is injected into that root process.

If there is no main process that serves as a root (with debugger running in it), then it becomes difficult to know when the new processes gets spawned. There is no way from the debugger to tell this from outside of the newly spawned process. It could be a unrelated python process. Another issue is, how do we know that a debugging session is over? how long does it take from process terminating to a new process start? The only direction that could potentially work in this case is the reverse direction, that is the launched process some how starts debug session (this is not supported and not a small work item). I am not sure how that scenario will work with vscode server.

[karthiknadig]

My original question is still un-answered. Is there a process that you have that restarts the server? if yes then is that process a python process? if yes then can that process itself be run under the debugger?
if no then we need to understand the architecture in greater detail to provide a solution.

[int19h]

Note that the original Node.js scenario that is referenced is an attach scenario - the user explicitly runs some code under nodemon, and attaches to that code. When some file in the monitored directories change, nodemon restarts the whole process.

So if I'm understanding the feature request correctly, it's a debug configuration property that would cause the debugger to immediately try to re-attach if the debug session ends, using the same exact debug config.

Note however that this is not idiomatic in the Python ecosystem. That is, there's no generic nodemon-like tool that is idiomatically used in such circumstances. Instead, individual frameworks, such as Django and Flask, implement their own file system watchers with code reloading, as @karthiknadig described above. So, for the Python debugger, our support for such reloading amounts simply to supporting multiprocess Python applications, and letting the parent process restart the child processes as needed. If you implement your nodemon equivalent for Azure Functions in this way, then we will automatically support it.

[ejizba]

Is there a process that you have that restarts the server? if yes then is that process a python process? if yes then can that process itself be run under the debugger? if no then we need to understand the architecture in greater detail to provide a solution.

Our scenario is for debugging Azure Functions, which supports several languages (Python, C#, JavaScript, etc.). The core runtime is built in .NET Core and that is what handles managing and/or restarting the various language-specific processes (what they call "workers"). So no, I don't think we can just attach to a "root" process because it is not implemented in Python.

So if I'm understanding the feature request correctly, it's a debug configuration property that would cause the debugger to immediately try to re-attach if the debug session ends, using the same exact debug config.

Yes, but when you say "if the debug session ends" the Node.js debugger seems to know the difference between "the debug port unexpectedly closed" vs "the user manually detached" and will not try to reattach in the latter scenario.

If there is no main process that serves as a root (with debugger running in it), then it becomes difficult to know when the new processes gets spawned.

As @int19h mentioned, we're focused specifically on the attach scenario. I expect the same timeout/waiting logic would apply for the first attach vs. the second attach.

[int19h]

I had a look at how the Node debug adapter does this. It seems to boil down to using "restart" in the "terminated" event, the presence of which causes VSCode to restart the session and pass the value of "restart" in the new debug config as "__restart". And the adapter only does it if the session ends by itself, and not if it receives a "terminate" request.

It sounds like something we could implement fairly easily in debugpy following the same pattern.

[karthiknadig]

Moving this to debugpy, since this is can be entirely handled there.

[karthiknadig]

@ejizba @nturinski thanks for the details

[fabioz]

I'll start to work on this issue.

I'll rephrase what I understood (so that someone can correct me if I got the wrong idea).

The idea is that given an attach launch configuration that has "restart": true, if the target process disconnects abruptly (i.e.: because it was killed), the adapter should ask VSCode to try to rerun the same attach launch configuration (by sending a "terminated" event with a "restart": <some_value>, which will be later received in the new attach request as "__restart").

[karthiknadig]

@fabioz That is correct.

[fabioz]

To give some feedback, I tried just passing the "restart": "restart" in the "terminated" event, but there are some complications...

The debug adapter in this case is started by the server (python process) as a separate process and starts communicating with VSCode at the specified port (say 5678) and then creates a separate connection to talk to the server process at a different random port (say 78787).

When it sends the "terminated" event with a "restart": "restart", the debug adapter is kept alive and VSCode keeps communicating with it (it wasn't started by VSCode in the first place, so, that seems fair, VSCode can only stop what was on his side -- but I was expecting the socket connection to be terminated, which wasn't the case).

This means that the new connection made fails when trying to start a new adapter to listen in the specified port (5678) and a new connection can't be made to the existing adapter because we don't know its port (it's also in a bit of a weird place where it already waited for servers.wait_until_disconnected() and is waiting for sessions.wait_until_ended()).

I tried sending the "terminated" with "restart" and closing the connection/exiting right afterwards but then I couldn't see VSCode connecting back on the newly spawned adapter.

Also, I can see that if the adapter process is not killed, VSCode itself keeps the connection open and issues a new attach request to it.

This doesn't work due to 2 things:

1. the new listen raises an exception because it'll try to spawn a new adapter and open a new server connection in a port that is already open (5678)).
2. even if we didn't create a new adapter, we don't know with which port we should connect to talk to the existing adapter.

So, I need to investigate a bit more on why VSCode is trying to reuse that connection (I thought that VSCode would just disconnect from 5678 -- in which case the adapter should exit and things should just work), but it seems this wasn't the case.

[fabioz]

I finished my investigation on this... Fixing this will need changes on both debugpy as well as vscode-python.

What was happening is that debugpy kept the server socket open after the initial connection, and vscode-python ended up connecting back to the old adapter (it's expected that it's kept open so that the user can disconnect/reconnect multiple times after the initial attach). Closing the server socket / exiting the adapter on the debugpy side does the proper thing.

The problem is that later on, if the connection isn't readily available (which happens because the process that'll listen again takes a bit of time to start-up and get to debugpy.listen again), vscode-python will not try to reconnect given a timeout.

i.e.: in: https://github.com/microsoft/vscode-python/blob/2021.6.944021595/src/client/debugger/extension/adapter/factory.ts#L50, it uses a vscode.DebugAdapterServer and it doesn't seem to have any option to try to reconnect or give any timeout, so, the connection ends up failing because the new connection isn't promptly available.

So, given that I'll proceed to do the changes on debugpy, but I'm still unsure of the best way to fix this on the vscode-python... ideally there'd be a readily-available way to create a vscode.DebugAdapterServer that tries to reconnect if the connection isn't readily available (as a note, I can't see any way to differentiate that the connection in https://github.com/microsoft/vscode-python/blob/2021.6.944021595/src/client/debugger/extension/adapter/factory.ts#L50 is doing a restart or not, so, this would probably also change the regular attach mode).

[fabioz]

Note: I'll have leave this on hold for a bit...

Some mental notes for when I (or someone else) checks it:

1. debugpy.adapter.clients.stop_serving() must be called before the terminate with restart is sent to the client so that the old adapter does not respond to the connection.
2. We should add a test with a case with multiprocessing when multiple connections are done (and thus terminate with restart should only be sent after the last one exits).
3. We shouldn't call debugpy.adapter.clients.stop_serving() in the regular case (when the VSCode disconnects), only in the case where all processes exit and the user used an attach launch configuration with restart as in the regular case the user can connect back to the same socket.
4. "terminated" is currently called at debugpy.adapter.sessions.Session._finalize, but we need to be careful on the heuristics for which "restart" should be passed (it may need more info than it has now).
5. Currently in debug mode in vscode-python it's possible to make it work by adding a breakpoint to https://github.com/microsoft/vscode-python/blob/2021.6.944021595/src/client/debugger/extension/adapter/factory.ts#L50 and letting it run only after a new connection is set up in debugpy.

[int19h]

I actually wonder if we can make this work without respawning the adapter, rather than trying to hack around the inherent problems with asynchronously connecting to it. So far as I can tell, there are two problems with this in the existing implementation:

1. The adapter doesn't expect a connection to be reused after receiving "disconnect" on it. Some parts of our implicit state machine that handles "initialize", "configurationDone" etc will likely break.

2. When the debuggee is restarted, it'll try to call debugpy.listen() again, which always tries to spawn a new adapter on the specified port, and fails if something else is already using that port. There's no way for the debug server to figure out that an existing adapter is already listening on that port, and reconnect to it.

The first problem should be straightforward to handle by resetting all the affected state. The second one is trickier, but I think it's possible for the new server and the original adapter to coordinate. While the new server doesn't know the port on which it should connect to the adapter as a server, it does know the port on which the adapter will be listening for clients, since that's passed to debugpy.listen() as an argument. If we add a custom client request to retrieve endpoint info, the server could connect to the adapter as a client first, and use that custom request to get the other port, and then connect to it normally.

For that matter, separate ports might not even be necessary. We currently use them to distinguish incoming client and server connections, but this can also be done based on the first message received on it - the server will simply send a custom one identifying it as such, and the adapter listener will then create an instance of Client or Server to process that connection as needed. In this case, the server will always have the necessary info to reconnect.

[int19h]

We also need to consider the scenario where there are two independent but overlapping debug sessions, each with its own adapter. If the user forgets to specify different port numbers, currently, this will fail and report an error when they try to start the second server. But if we do the above, it will end up quietly attaching to the adapter from the first session, and the whole thing will be treated as one large multiproc session, which would be rather surprising.

The problem is that the adapter is unable to distinguish between the servers to figure out if the server connecting to it is a new instance that logically corresponds to another session that ended earlier, or it's a completely new one. If the actual restart process provided some facility to pass information from the old server to the new one (like "__restart" does for clients), we could use that to maintain some sort of persistent ID. If there's no such facility, I think the ID would have to be provided externally; that is, we'd have to add the ability to pass it to --listen / debugpy.listen(), and the reloader daemon would have to generate that ID when it starts the server for the first time, and consistently pass it on all restarts.

Or perhaps this is best handled by using --connect instead? In that scenario, the adapter port to which the server connects isn't random, but is specified by the user. So when the server process restarts with the same arguments, it'd automatically reconnect to the same adapter; and --listen wouldn't support restarts. @ejizba, would such a constraint be acceptable for your scenario?

[snake-py]

So as someone who just came here - I see you guys are all the way in a discussion about how it could be implemented and seeing the last update is a year ago - I thought I check where the feature request is at.

I am running Django inside docker and it is an attached process. As an outsider, I do not see why this causes the debuggy process to close.

Or should this already work and my configs are just wrong?

[int19h]

We still don't support "restart", so if your scenario relies on that, it would be expected to not work.

[fabioz]

@int19h I was thinking about taking a look into the approach of doing the restart while keeping the debug adapter process live. Is this Ok or do you have plans to work on that already?

[int19h]

@fabioz I have some WIP code for this already.

[int19h]

Based on the discussion above and in #1212, I'm going to handle this piecemeal. Specifically, the "attach"{"listen"} scenario (i.e. where the debuggee connects to the client) will be first - it's much simpler and with fewer moving bits to synchronize because VSCode is responsible for adapter lifetime in that case; yet it also seemingly covers all the practical scenarios. The complexity induced by the requirements of doing this for "attach"{"connect"} (i.e. when the debuggee is listening) does not feel like it's worth it, and it's not clear wrt "launch", either.

[mrlowe]

Hello from 2024! Seeing as how #577 has been closed one might wonder if the changes mentioned by @int19h were ever implemented and/or released. I assume not because there's no update here, but it would be nice if I were wrong. Anything new on this front?