Wait until disconnect request is received and replied to when debugee terminates #134
Conversation
| @@ -507,7 +514,15 @@ def on_configurationDone(self, request, args): | |||
|
|
|||
| def on_disconnect(self, request, args): | |||
| # TODO: docstring | |||
| self.send_response(request) | |||
| if self.start_reason == 'launch': | |||
There was a problem hiding this comment.
I think terminateDebuggee argument should be handled here as well.
There was a problem hiding this comment.
terminateDebuggee
We're not supporting that are we?
There was a problem hiding this comment.
I think we should handle that as a separate PR, that's a new feature (new capability that we need to expose and I don't see that being exposed in on_initialize
There was a problem hiding this comment.
VS supports 'Detach' from a launched application. See #86. We can handle that in a separate change, if it is not necessary for VSC.
There was a problem hiding this comment.
Created a separate issue for that #136
| @@ -413,6 +415,11 @@ def close(self): | |||
| self.send_event('exited', exitCode=ptvsd_sys_exit_code) | |||
| self.send_event('terminated') | |||
|
|
|||
| self.disconnect_request_event.wait(WAIT_FOR_DISCONNECT_REQUEST_TIMEOUT) | |||
| if self.disconnect_request is not None: | |||
There was a problem hiding this comment.
if self.disconnect_request_event.isSet(): do we need disconnect_request ?
There was a problem hiding this comment.
Not following you.
There was a problem hiding this comment.
I updated the comment. I think the event state is enough to determine if there was a disconnect request.
There was a problem hiding this comment.
I misunderstood this part ignore my comment.
|
This fixes #28 |
ptvsd/wrapper.py
Outdated
| self.send_response(request) | ||
| if self.start_reason == 'launch': | ||
| self.disconnect_request_event.set() | ||
| self.disconnect_request = request |
There was a problem hiding this comment.
This should assign the field first, then set the event.
There was a problem hiding this comment.
oh yes, bug, thanks
ptvsd/wrapper.py
Outdated
| self.disconnect_request = request | ||
| killProcess = not self._closed | ||
| self.close() | ||
| if killProcess: |
There was a problem hiding this comment.
Why does the process need to kill itself like that?
There was a problem hiding this comment.
sys.exit(0), doesn't kill the process. it just runs it to completion.
E.g. if I were to add a breakpoint in a loop, and then stop the debugger (sys.exit(0) at this point doesn't end the loop/process, basically the program continues)
|
Please could someone merge this for me. |
|
@DonJayamanne looks like there are some merge conflicts. Let me know when you are done resolving them. |
|
Easy, will do tonight. |
|
done, merge issues resolved. |
Fixes #120
Fixes #130
This has a dependency on #129