Shutdown TCP ServerSocket when parent process ends (#655)

Author: andfoy

appveyor.yml

@@ -1,4 +1,6 @@
 environment:
+  global:
+    APPVEYOR_RDP_PASSWORD: "dcca4c4863E30d56c2e0dda6327370b3#"
   matrix:
     - PYTHON: "C:\\Python27"
       PYTHON_VERSION: "2.7.15"
@@ -21,6 +23,9 @@ install:
 test_script:
   - "%PYTHON%/Scripts/pytest.exe test/"
 
+# on_finish:
+#   - ps: $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))
+
 build: false  # Not a C# project
 
 cache:

pyls/python_ls.py

@@ -34,18 +34,28 @@ def setup(self):
 
     def handle(self):
         self.delegate.start()
+        # pylint: disable=no-member
+        self.SHUTDOWN_CALL()
 
 
 def start_tcp_lang_server(bind_addr, port, check_parent_process, handler_class):
     if not issubclass(handler_class, PythonLanguageServer):
         raise ValueError('Handler class must be an instance of PythonLanguageServer')
 
+    def shutdown_server(*args):
+        # pylint: disable=unused-argument
+        log.debug('Shutting down server')
+        # Shutdown call must be done on a thread, to prevent deadlocks
+        stop_thread = threading.Thread(target=server.shutdown)
+        stop_thread.start()
+
     # Construct a custom wrapper class around the user's handler_class
     wrapper_class = type(
         handler_class.__name__ + 'Handler',
         (_StreamHandlerWrapper,),
         {'DELEGATE_CLASS': partial(handler_class,
-                                   check_parent_process=check_parent_process)}
+                                   check_parent_process=check_parent_process),
+         'SHUTDOWN_CALL': shutdown_server}
     )
 
     server = socketserver.TCPServer((bind_addr, port), wrapper_class)
@@ -78,6 +88,7 @@ def __init__(self, rx, tx, check_parent_process=False):
         self.workspace = None
         self.config = None
         self.root_uri = None
+        self.watching_thread = None
         self.workspaces = {}
         self.uri_workspace_mapper = {}
 
@@ -187,19 +198,18 @@ def m_initialize(self, processId=None, rootUri=None, rootPath=None, initializati
         self._dispatchers = self._hook('pyls_dispatchers')
         self._hook('pyls_initialize')
 
-        if self._check_parent_process and processId is not None:
+        if self._check_parent_process and processId is not None and self.watching_thread is None:
             def watch_parent_process(pid):
                 # exit when the given pid is not alive
                 if not _utils.is_process_alive(pid):
                     log.info("parent process %s is not alive", pid)
                     self.m_exit()
-                log.debug("parent process %s is still alive", pid)
-                threading.Timer(PARENT_PROCESS_WATCH_INTERVAL, watch_parent_process, args=[pid]).start()
-
-            watching_thread = threading.Thread(target=watch_parent_process, args=(processId,))
-            watching_thread.daemon = True
-            watching_thread.start()
+                else:
+                    threading.Timer(PARENT_PROCESS_WATCH_INTERVAL, watch_parent_process, args=[pid]).start()
 
+            self.watching_thread = threading.Thread(target=watch_parent_process, args=(processId,))
+            self.watching_thread.daemon = True
+            self.watching_thread.start()
         # Get our capabilities
         return {'capabilities': self.capabilities()}
 

test/test_language_server.py

@@ -1,5 +1,7 @@
 # Copyright 2017 Palantir Technologies, Inc.
 import os
+import time
+import multiprocessing
 from threading import Thread
 
 from test import unix_only
@@ -8,7 +10,7 @@
 
 from pyls.python_ls import start_io_lang_server, PythonLanguageServer
 
-CALL_TIMEOUT = 2
+CALL_TIMEOUT = 10
 
 
 def start_client(client):
@@ -23,11 +25,12 @@ def __init__(self, check_parent_process=False):
         # Server to client pipe
         scr, scw = os.pipe()
 
-        self.server_thread = Thread(target=start_io_lang_server, args=(
+        ParallelKind = multiprocessing.Process if os.name != 'nt' else Thread
+
+        self.process = ParallelKind(target=start_io_lang_server, args=(
             os.fdopen(csr, 'rb'), os.fdopen(scw, 'wb'), check_parent_process, PythonLanguageServer
         ))
-        self.server_thread.daemon = True
-        self.server_thread.start()
+        self.process.start()
 
         self.client = PythonLanguageServer(os.fdopen(scr, 'rb'), os.fdopen(csw, 'wb'), start_io_lang_server)
         self.client_thread = Thread(target=start_client, args=[self.client])
@@ -56,9 +59,10 @@ def client_exited_server():
     """
     client_server_pair = _ClientServer(True)
 
-    yield client_server_pair.client
+    # yield client_server_pair.client
+    yield client_server_pair
 
-    assert client_server_pair.server_thread.is_alive() is False
+    assert client_server_pair.process.is_alive() is False
 
 
 def test_initialize(client_server):  # pylint: disable=redefined-outer-name
@@ -72,12 +76,17 @@ def test_initialize(client_server):  # pylint: disable=redefined-outer-name
 @unix_only
 def test_exit_with_parent_process_died(client_exited_server):  # pylint: disable=redefined-outer-name
     # language server should have already exited before responding
-    with pytest.raises(Exception):
-        client_exited_server._endpoint.request('initialize', {
-            'processId': 1234,
-            'rootPath': os.path.dirname(__file__),
-            'initializationOptions': {}
-        }).result(timeout=CALL_TIMEOUT)
+    lsp_server, mock_process = client_exited_server.client, client_exited_server.process
+    # with pytest.raises(Exception):
+    lsp_server._endpoint.request('initialize', {
+        'processId': mock_process.pid,
+        'rootPath': os.path.dirname(__file__),
+        'initializationOptions': {}
+    }).result(timeout=CALL_TIMEOUT)
+
+    mock_process.terminate()
+    time.sleep(CALL_TIMEOUT)
+    assert not client_exited_server.client_thread.is_alive()
 
 
 def test_not_exit_without_check_parent_process_flag(client_server):  # pylint: disable=redefined-outer-name