Repository navigation
Support restart in terminated event in debug adapter #577
Description
Activity
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.
Reacted by FabioHello, 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!
Reacted by NathanAdding +1 on this - we have many customers in Azure Functions Python VSCode experience
Hello Karthik Nadig (@karthiknadig), do you think there is already enough data points for this work item? Thanks!
Stefanus Hinardi (@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?
Reacted by Pavel MinaevPinging Nathan (@nturinski) and RogerZeng (@Hazhzeng) who are the experts on this
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.
Nathan (@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 likedjangowhich handles re-load on save. They way it works there is, the maindjangoprocess is run under the debugger.djangothen 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,djangodetects 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 thedjangomain 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.
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.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,nodemonrestarts 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 Karthik Nadig (@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 yournodemonequivalent for Azure Functions in this way, then we will automatically support it.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 Pavel Minaev (@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.
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.
- changed the title
[-]Add restart property to launch.json configurations[/-][+]Support `restart` in terminated event[/+]on Mar 29, 2021 - changed the title
[-]Support `restart` in terminated event[/-][+]Support `restart` in terminated event in debug adapter[/+]on Mar 29, 2021 Moving this to
debugpy, since this is can be entirely handled there.Reacted by Nathan and Stefanus Hinardi10 remaining items
Note: I'll have leave this on hold for a bit...
Some mental notes for when I (or someone else) checks it:
debugpy.adapter.clients.stop_serving()must be called before theterminatewithrestartis sent to the client so that the old adapter does not respond to the connection.- We should add a test with a case with multiprocessing when multiple connections are done (and thus
terminatewithrestartshould only be sent after the last one exits). - 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 anattachlaunch configuration withrestartas in the regular case the user can connect back to the same socket. "terminated"is currently called atdebugpy.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).- Currently in debug mode in
vscode-pythonit'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 indebugpy.
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:
-
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.
-
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
ClientorServerto process that connection as needed. In this case, the server will always have the necessary info to reconnect.-
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
--connectinstead? 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--listenwouldn't support restarts. Eric Jizba (@ejizba), would such a constraint be acceptable for your scenario?- assigned and unassigned
on Sep 15, 2021 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?
We still don't support "restart", so if your scenario relies on that, it would be expected to not work.
Pavel Minaev (@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?
Fabio Zadrozny (@fabioz) I have some WIP code for this already.
Reacted by Fabio Zadrozny and Sven SoostBased 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.
- locked and limited conversation to collaborators
on Feb 16, 2023
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.