Skip to content
This repository has been archived by the owner on Aug 2, 2023. It is now read-only.

Wait until disconnect request is received and replied to when debugee terminates #134

Merged
merged 4 commits into from
Feb 28, 2018
Merged

Wait until disconnect request is received and replied to when debugee terminates #134

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1456712
wait for disconnect request when terminating
DonJayamanne Feb 27, 2018
d4acd60
SIGHUP is not available on windows
DonJayamanne Feb 27, 2018
16ea17a
fix code review
DonJayamanne Feb 27, 2018
5431ace
merged master
DonJayamanne Feb 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions ptvsd/wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import errno
import os
import platform
import signal
import socket
import sys
import threading
Expand Down Expand Up @@ -37,7 +38,7 @@
#ipcjson._TRACE = ipcjson_trace

ptvsd_sys_exit_code = 0

WAIT_FOR_DISCONNECT_REQUEST_TIMEOUT = 2

def unquote(s):
if s is None:
Expand Down Expand Up @@ -389,7 +390,8 @@ def __init__(self, socket, pydevd, logfile=None):
self.next_var_ref = 0
self.loop = futures.EventLoop()
self.exceptions_mgr = ExceptionsManager(self)

self.disconnect_request = None
self.disconnect_request_event = threading.Event()
pydevd._vscprocessor = self
self._closed = False

Expand All @@ -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:
Copy link
Member

@karthiknadig karthiknadig Feb 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if self.disconnect_request_event.isSet(): do we need disconnect_request ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not following you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the comment. I think the event state is enough to determine if there was a disconnect request.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood this part ignore my comment.

self.send_response(self.disconnect_request)
self.disconnect_request = None

self.loop.stop()

if self.socket:
Expand Down Expand Up @@ -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':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think terminateDebuggee argument should be handled here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

terminateDebuggee

We're not supporting that are we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS supports 'Detach' from a launched application. See #86. We can handle that in a separate change, if it is not necessary for VSC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a separate issue for that #136

self.disconnect_request_event.set()
self.disconnect_request = request
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should assign the field first, then set the event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yes, bug, thanks

killProcess = not self._closed
self.close()
if killProcess:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the process need to kill itself like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

os.kill(os.getpid(), signal.SIGTERM)
else:
self.send_response(request)

@async_handler
def on_attach(self, request, args):
Expand Down Expand Up @@ -931,6 +946,10 @@ def _start(client, server):
########################
# pydevd hooks

def signal_handler(signum, frame, proc):
proc.close()
sys.exit(0)

def start_server(port):
"""Return a socket to a (new) local pydevd-handling daemon.

Expand All @@ -943,6 +962,8 @@ def start_server(port):
client, _ = server.accept()
pydevd, proc, _ = _start(client, server)
atexit.register(proc.close)
if platform.system() != 'Windows':
signal.signal(signal.SIGHUP, lambda signum, frame: signal_handler(signum, frame, proc))
return pydevd


Expand All @@ -958,6 +979,8 @@ def start_client(host, port):
client.connect((host, port))
pydevd, proc, _ = _start(client, None)
atexit.register(proc.close)
if platform.system() != 'Windows':
signal.signal(signal.SIGHUP, lambda signum, frame: signal_handler(signum, frame, proc))
return pydevd


Expand Down