Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try soft-kill before hard-kill when terminating debugger #1022

Closed
tjnd89 opened this issue Aug 18, 2022 · 8 comments
Closed

Try soft-kill before hard-kill when terminating debugger #1022

tjnd89 opened this issue Aug 18, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@tjnd89
Copy link

tjnd89 commented Aug 18, 2022

Type: Bug

I've observed a strange behavior where VSCode does not release COM object references while debugging. PyCharm works fine. To see this in action do the following:

Start Excel (the problem occurs with other apps too, I'm using Excel for this example only because most people have it).

From the command line, run the script "excel_test.py" below. It attaches to the running Excel application and tries to call a non-existent method hi(). An exception is thrown and the script will exit, as expected.

import win32com.client as w32c

# Attach to a running Excel
app = w32c.GetActiveObject("Excel.Application")

# This will throw an exception
app.hi()

print("done")

Quit Excel manually by closing it via the UI. Note in the task manager that the Excel.exe process has shut down. This is all as expected.

Repeat the same steps but run the script using VSCode debug mode. When the exception is thrown the debugger halts execution. Press the Stop button on the debugger tool bar to terminate the script. After quitting Excel, notice that the UI has shutdown, but the Excel.exe process is still running. You now have a zombie process.

Following this same pattern in PyCharm debug mode works fine, the process goes away as expected.

Somehow it appears that VSCode is not properly releasing the COM object reference when exiting debug mode. Am I missing some weird setting or something, or is it just broken?

I'm running Python 3.9.12, pywin32 302, VSCode 1.70.2.

I've also opened up a StackOverflow question on this at https://stackoverflow.com/questions/73401543/vscode-debugger-not-releasing-com-object-reference-when-a-python-script-throws-a.

Extension version: 2022.12.1
VS Code version: Code 1.70.2 (e4503b30fc78200f846c62cf8091b76ff5547662, 2022-08-16T05:35:13.448Z)
OS version: Windows_NT x64 10.0.22000
Modes:

System Info
Item Value
CPUs Intel(R) Core(TM) i9-10900K CPU @ 3.70GHz (20 x 3696)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 31.90GB (17.48GB free)
Process Argv C:\Users\tj\OneDrive - Exa Research\Training\ODTK\Integrating\Exercises --crash-reporter-id 07d7cc17-f9d3-4972-9a41-087017e04f32
Screen Reader no
VM 0%
@karthiknadig karthiknadig transferred this issue from microsoft/vscode-python Aug 19, 2022
@fabioz
Copy link
Collaborator

fabioz commented Aug 19, 2022

@tjnd89 I don't really have excel here to test, but we should be killing the whole process tree (so, if excel is not part of the process tree this could explain it).

Can you do one test: if you use taskkill directly by running the command line below (replacing the <pid> with the actual pid of the running process):

taskkill /F /PID <pid> /T

Does it work?

Note: you can get the pid by adding the code below in your script.

import os
print(os.getpid())

@tjnd89
Copy link
Author

tjnd89 commented Aug 19, 2022

@fabioz I tried your test. The taskkill said the process was not found - since the PID is the one for the Python script and it's been terminated. Once the script has died in the debugger and I've told the debugger to halt, quitting Excel via the UI still leaves the Excel zombie process.

Excel is not part of the script's process tree since it was originally started before even running the script.

In the non-debugger case, the app variable goes out of scope and releases the COM object reference. So I think the goal is to ensure that when the debugger is halting that it ensures all variables go out of scope properly.

@fabioz
Copy link
Collaborator

fabioz commented Aug 19, 2022

I meant for you to use taskkill instead of terminating while you're in the unhandled exception...

Please try it with and without the /F to see if it makes any difference.

@tjnd89
Copy link
Author

tjnd89 commented Aug 19, 2022

Gotcha. I just tried again.

  1. I let the debugger catch the exception. Then I ran taskkill with /F and it successfully terminated the script. But exiting Excel via the UI still left a zombie process.

  2. I repeated the test without /F, in that case nothing happens in the debugger - it's still waiting for me to interact with it. I have to tell it to stop the script. Quitting Excel via the UI still left a zombie process.

@fabioz
Copy link
Collaborator

fabioz commented Aug 19, 2022

One more try: what happens if instead of doing the kill you go to the terminal and make a Ctrl+C?

I guess one difference in our implementation is that we just force kill and I believe in Intellij they try a soft-kill first through a Ctrl+C.

@fabioz fabioz changed the title Debugger does not release COM object references Try soft-kill before hard-kill when terminating debugger Aug 19, 2022
@fabioz fabioz added the enhancement New feature or request label Aug 19, 2022
@tjnd89
Copy link
Author

tjnd89 commented Aug 19, 2022

Ctrl-C in the terminal successfully stopped the debugger, then quitting Excel via it's UI showed that the Excel process went away. So it looks like the soft-kill did the right thing.

fabioz added a commit to fabioz/debugpy that referenced this issue Aug 19, 2022
@fabioz
Copy link
Collaborator

fabioz commented Aug 19, 2022

It seems that to properly do this we should support: supportsTerminateRequest.

i.e.: The spec says:

The terminate request is sent from the client to the debug adapter in order to shut down the debuggee gracefully. Clients should only call this request if the capability supportsTerminateRequest is true.
Typically a debug adapter implements terminate by sending a software signal which the debuggee intercepts in order to clean things up properly before terminating itself.
Please note that this request does not directly affect the state of the debug session: if the debuggee decides to veto the graceful shutdown for any reason by not terminating itself, then the debug session just continues.
Clients can surface the terminate request as an explicit command or they can integrate it into a two stage Stop command that first sends terminate to request a graceful shutdown, and if that fails uses disconnect for a forceful shutdown.

@fabioz
Copy link
Collaborator

fabioz commented Aug 19, 2022

@int19h @karthiknadig I've created #1023 as a draft as a possible approach to address this...

fabioz added a commit to fabioz/debugpy that referenced this issue Sep 2, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Sep 2, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Sep 2, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Sep 8, 2022
fabioz added a commit to fabioz/debugpy that referenced this issue Sep 8, 2022
@fabioz fabioz closed this as completed in 8157273 Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants