Fix for error message on program exit #25
Conversation
ptvsd/wrapper.py
Outdated
| @@ -700,6 +717,8 @@ def start_server(port): | |||
| server_thread.daemon = True | |||
| server_thread.start() | |||
|
|
|||
| atexit.register(lambda : proc.close()) | |||
There was a problem hiding this comment.
You don't need a lambda here anymore - you can just pass proc.close directly.
ptvsd/wrapper.py
Outdated
| def __init__(self): | ||
| self.prog_exit_code = 0 | ||
| self._sys_exit = sys.exit | ||
| sys.exit = self.exit_hook |
There was a problem hiding this comment.
The problem with this approach is that it's not going to be particularly reliable. Suppose someone did from sys import exit - now they have a copy of what sys.exit was as a global or local variable at the time of the import, and if we haven't detoured yet, changing sys.exit inside the module isn't going to change that copy. Also, Python extensions can exit the process too, and they wouldn't invoke sys.exit (but rather the native function that backs it).
I'm not sure there's a better approach though. @brettcannon, can you think of anything? Basically we need to somehow capture the exit code of the process - or rather what we think said code will be - before it exits (because the code used to report the exit lives in that process).
There was a problem hiding this comment.
sys.exit() just raises SystemExit, so if you can grab that and check the exit code it's carrying that would be more reliable. (Maybe register something using the atexit module and then see whether SystemExit is set?)
There was a problem hiding this comment.
You mean, look at sys.exc_info()? I just tried it, and looks like it's already cleared by the time atexit functions are running.
For launch scenarios, we could trap SystemExit at our entry point - provided that pydevd doesn't catch it first? Need to test whether this is feasible. For attach, though, this won't work, because user code is already running by the time it happens.
OTOH, the old debugger doesn't report exit code correctly in attach scenario, either, so it could be good enough.
There was a problem hiding this comment.
Yep, I was thinking of sys.exc_info().
ptvsd/wrapper.py
Outdated
| def exit_hook(self, code=0): | ||
| self.prog_exit_code = code | ||
| self._sys_exit(code) | ||
| __ptvsd_sys_exit_code = 0 |
There was a problem hiding this comment.
You can just make the variable public, and set and read it directly, even across the module boundary - in Python, it's idiomatic to skip accessor methods if all they do is read & write.
No description provided.