Terminate should try to do a graceful shutdown. Fixes #1022

[fabioz]

Note: this is still a draft.

I'd like opinions on whether this would be the best approach @int19h @karthiknadig.

Also note that the graceful shutdown can be prevented, so, the debugger should probably remain active, but when the KeyboardInterrupt exception escapes the tracing hook, the debugger effectively becomes disabled because python sets the tracing to None (so, we still need to see how to deal with that -- in frame eval mode it should be possible to re-enable it on a new breakpoint hit, but in tracing mode this may be harder).

Maybe something with PyThreadState_SetAsyncExc could get the tracing back in some way.

[int19h]

I wonder if we can just call sys.exit() here and expect reliable cleanup?

[fabioz]

I don't think it's much better.

sys.exit will also raise an exception, but it will only work if called from the main thread, which is harder to do (Python has support for interrupting with a KeyboardInterrupt in the main thread, but doing a sys.exit() -- or effectively raising a SystemExit in the main thread in the case where the process isn't in a breakpoint is harder, especially if it's in some sleep or io operation).

And it still has the same issue where the user can catch the exception...

[int19h]

@brettcannon Do you have any advice on how to best do graceful but (ideally) non-cancelable shutdown in CPython?

[brettcannon]

Best way to exit is https://docs.python.org/3/library/sys.html#sys.exit , but that can be interrupted. Hard exit that can't be stopped is https://docs.python.org/3/library/os.html#os._exit . There is no in-between because clean-up code during shutdown will have an opportunity to halt things if necessary.

[fabioz]

The problem is that to exit in this case the sys.exit would need to be called in the main thread and not in a secondary thread to work, so, I think it'd be more reliable to send a Ctrl+C to interrupt the main thread (also, both SystemExit as well as KeyboardInterrupt may still be handled by the user).

If we want a soft-kill with a graceful shutdown from the application I think a Ctrl+C is usually more expected from users that want a clean shutdown.

I think that the major thing we need to note for users is that if that's done while stopped in a breakpoint the debugging will be unavailable from that point onwards in that thread (but we do already have this issue if the user does a Ctrl+C, so, initially we should at least warn the user that this happened).

We should probably make this a debugger flag (such as terminateSoftKill being initially false) and roll it out slowly.

Does creating a terminateSoftKill, notifying user that thread in no longer traced if that's the case and doing a Ctrl+C interrupt sound reasonable? @int19h @karthiknadig

[brettcannon]

FYI KeyboardInterrupt can still be caught and suppressed (albeit rarely since it inherits from BaseException and not Exception).

[fabioz]

Ok, this pull request has the following changes now:

1. A terminateSoftKill option is now available when launching (default is false -- I think we shouldn't make it true as in general a hard kill is usually what's expected in most cases, but the user can at least choose now).
2. If "terminateSoftKill": true is set, the main thread is interrupted with a KeyboardInterrupt so that he can do the needed shutdown cooperating with the application (the client is expected to do a disconnect afterwards if the user presses the terminate button a 2nd time to do a hard kill -- this is written in the spec and is what VSCode does).
3. If an exception leaves the tracing an output event (marked console) notifying about that is sent.

[int19h]

Looks great, but rather than making it a Boolean flag, I think it would be more consistent with other debug options to have an enum here. So the default would be:

"onTerminate": "kill";

but users could set:

"onTerminate": "KeyboardInterrupt";

Makes it a bit more readable, and if we get asked for other methods, we can easily add them.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

60.0% Coverage
0.0% Duplication

[johnbendi]

Please where is more information on this feature documented as I need to hook into the debugger restart process. My atexit hooks are not called. And I can't reach the KeyboardInterrupt from my code as the third party library is not rethrowing it.

[int19h]

It's not designed as an extensibility point; the idea for this change was to allow the code in the process to clean up as it normally would during process shutdown, but it's not supposed to know that the shutdown is happening because the user clicked "Stop debugging" in their IDE. Thus all specifics are implementation details, and there's no actual guarantee that you'll get a graceful shutdown in any case - it's still best-effort, and the process still gets hard-killed if simulating Ctrl+C doesn't shut it down fast enough.

If you want to customize the debugger workflow, you might have to intercept the DAP message stream and detect "disconnect" and "terminate" requests there to do custom processing (e.g. notifying your process to do something before propagating the message to the debug server, or even replacing it with a different message if you want to block termination) in your proxy. This can be done in different ways; if you're using VSCode, the easiest would probably be https://code.visualstudio.com/api/references/vscode-api#DebugAdapterTracker