Fix debugged process not being terminated when debugging session closes on Linux #620
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See discussion near godotengine#613 (comment) . Currently, the debugger process is spawned with `shell: true` option, which means a shell process is launched first, and its PID is returned. When a debugging session terminates, `godot-tools` attempts to kill the process. However, on Linux, calling `process.kill()` on that kills the shell process, while allowing the main process to keep running. To fix, enable `detached: true` flag when launching the debugger to force both processes to share the same process group; then rely on changes introduced in godotengine#613 to kill the entire group.
|
This solves the issue using Linux and Godot 3.5.3 |
|
@vpyatnitskiy Thanks for doing the legwork on this. |
|
I quickly tested |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up for #613. Both PRs need to be merged for this change to work.
Issue:
On Linux, when debugging session is initiated and then closed from VS Code, the game (i.e. the process being debugged) does not terminate.
Steps to reproduce are as you would expect:
Have a launch configuration for GDScript in VS Code properly set up.
Launch a project with F5.
Switch back to VS Code. Press Shift+F5 to close the debugging session.
The session closes but the project continues running.
Cause:
Similar to Fix child processes not being killed properly #613, the debugger process is started with
shell: trueflag, which causeschild_process.spawn()to first start a shell process which, in turn, starts the actual debugger. The returned PID is the shell's PID; the debugger runs under a different PID.When a debugging session terminates, the shell PID is killed. However, in Linux, child processes are allowed to persist when their parent is killed, so the debugger process survives.
Contrary to Fix child processes not being killed properly #613, the debugger is not launched with
detached: true― which means it is also running under its own process group, and is effectively inaccessible to VS Code.Solution:
Add
detached: trueto force both processes to share a process group. Rely on the change from #613 to kill the entire group rather than individual process(es).Other notes:
Unlike #613, this change is not restricted to Linux, and may potentially cause unintended consequences on Windows and/or Mac OS. This shouldn't be the case assuming headless LSP works properly on these platforms, but more extensive testing is necessary.
In case it does cause problems on other platforms, the suggested fix is to set the flag depending on the platform, e.g.
detached: process.platform !== "win32" && process.platform !== "darwin"or similar.Additionally, Godot 3 integration may need to be tested separately since it uses a separate code path.