Auto merge of #310 - micbou:cs-completer, r=Valloric

[READY] Prevent simultaneous starts of OmniSharp server

Currently, C# completer starts a new server on the `FileReadyToParse` event if no port is defined or the running server does not respond. This can lead to situations where multiple servers with same port and solution are started until one of them become ready. See PR #284 and ycm-core/YouCompleteMe#1913.

This is fixed by using `ServerIsActive` instead of `ServerIsRunning` to start the server in `OnFileReadyToParse`. We then check `ServerIsRunning` for an early return.

`ServerIsActive` is updated to check the handle (instead of the port) and to poll the process. We cannot only rely on the port because it can be defined while the process is down: server crashed during start up or its process was directly terminated by the user.

`ServerIsRunning` and `ServerIsReady` are also changed. We return early in both methods by checking `ServerIsAlive` first and we restrict exceptions catching.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/310)
<!-- Reviewable:end -->

ycmd/completers/cs/cs_completer.py

@@ -201,17 +201,17 @@ def GetSubcommandsMap( self ):
       'GetDoc'                           : ( lambda self, request_data, args:
          self._SolutionSubcommand( request_data,
                                    method = '_GetDoc' ) ),
-      'ServerRunning'                    : ( lambda self, request_data, args:
+      'ServerIsRunning'                  : ( lambda self, request_data, args:
          self._SolutionSubcommand( request_data,
                                    method = 'ServerIsRunning',
                                    no_request_data = True ) ),
-      'ServerReady'                      : ( lambda self, request_data, args:
+      'ServerIsHealthy'                  : ( lambda self, request_data, args:
          self._SolutionSubcommand( request_data,
-                                   method = 'ServerIsReady',
+                                   method = 'ServerIsHealthy',
                                    no_request_data = True ) ),
-      'ServerTerminated'                 : ( lambda self, request_data, args:
+      'ServerIsReady'                    : ( lambda self, request_data, args:
          self._SolutionSubcommand( request_data,
-                                   method = 'ServerTerminated',
+                                   method = 'ServerIsReady',
                                    no_request_data = True ) )
     }
 
@@ -227,11 +227,18 @@ def _SolutionSubcommand( self, request_data, method,
   def OnFileReadyToParse( self, request_data ):
     solutioncompleter = self._GetSolutionCompleter( request_data )
 
-    if ( not solutioncompleter.ServerIsRunning() and
-         self.user_options[ 'auto_start_csharp_server' ] ):
+    # Only start the server associated to this solution if the option to
+    # automatically start one is set and no server process is already running.
+    if ( self.user_options[ 'auto_start_csharp_server' ]
+         and not solutioncompleter.ServerIsRunning() ):
       solutioncompleter._StartServer()
       return
 
+    # Bail out if the server is unresponsive. We don't start or restart the
+    # server in this case because current one may still be warming up.
+    if not solutioncompleter.ServerIsHealthy():
+      return
+
     errors = solutioncompleter.CodeCheck( request_data )
 
     diagnostics = [ self._QuickFixToDiagnostic( x ) for x in
@@ -293,32 +300,20 @@ def DebugInfo( self, request_data ):
       return 'OmniSharp Server is not running'
 
 
-  def ServerIsRunning( self, request_data = None ):
-    """ Check if our OmniSharp server is running. """
-    return self._CheckSingleOrAllActive( request_data,
-                                         lambda i: i.ServerIsRunning() )
+  def ServerIsHealthy( self ):
+    """ Check if our OmniSharp server is healthy (up and serving). """
+    return self._CheckAllRunning( lambda i: i.ServerIsHealthy() )
 
 
-  def ServerIsReady( self, request_data = None ):
+  def ServerIsReady( self ):
     """ Check if our OmniSharp server is ready (loaded solution file)."""
-    return self._CheckSingleOrAllActive( request_data,
-                                         lambda i: i.ServerIsReady() )
+    return self._CheckAllRunning( lambda i: i.ServerIsReady() )
 
 
-  def ServerTerminated( self, request_data = None ):
-    """ Check if the server process has already terminated. """
-    return self._CheckSingleOrAllActive( request_data,
-                                         lambda i: i.ServerTerminated() )
-
-
-  def _CheckSingleOrAllActive( self, request_data, action ):
-    if request_data is not None:
-      solutioncompleter = self._GetSolutionCompleter( request_data )
-      return action( solutioncompleter )
-    else:
-      solutioncompleters = self._completer_per_solution.values()
-      return all( action( completer )
-        for completer in solutioncompleters if completer.ServerIsActive() )
+  def _CheckAllRunning( self, action ):
+    solutioncompleters = self._completer_per_solution.values()
+    return all( action( completer ) for completer in solutioncompleters
+                if completer.ServerIsRunning() )
 
 
   def _GetSolutionFile( self, filepath ):
@@ -402,7 +397,7 @@ def _StopServer( self ):
     self._TryToStopServer()
 
     # Kill it if it's still up
-    if not self.ServerTerminated() and self._omnisharp_phandle is not None:
+    if self.ServerIsRunning():
       self._logger.info( 'Killing OmniSharp server' )
       self._omnisharp_phandle.kill()
 
@@ -418,7 +413,7 @@ def _TryToStopServer( self ):
       except:
         pass
       for _ in range( 10 ):
-        if self.ServerTerminated():
+        if not self.ServerIsRunning():
           return
         time.sleep( .1 )
 
@@ -546,36 +541,31 @@ def _DefaultParameters( self, request_data ):
     return parameters
 
 
-  def ServerIsActive( self ):
-    """ Check if our OmniSharp server is active (started, not yet stopped)."""
-    try:
-      return bool( self._omnisharp_port )
-    except:
-      return False
+  def ServerIsRunning( self ):
+    """ Check if our OmniSharp server is running (process is up)."""
+    return utils.ProcessIsRunning( self._omnisharp_phandle )
 
 
-  def ServerIsRunning( self ):
-    """ Check if our OmniSharp server is running (up and serving)."""
+  def ServerIsHealthy( self ):
+    """ Check if our OmniSharp server is healthy (up and serving)."""
+    if not self.ServerIsRunning():
+      return False
+
     try:
-      return bool( self._omnisharp_port and
-                   self._GetResponse( '/checkalivestatus', timeout = .2 ) )
-    except:
+      return self._GetResponse( '/checkalivestatus', timeout = .2 )
+    except requests.exceptions.RequestException:
       return False
 
 
   def ServerIsReady( self ):
     """ Check if our OmniSharp server is ready (loaded solution file)."""
-    try:
-      return bool( self._omnisharp_port and
-                   self._GetResponse( '/checkreadystatus', timeout = .2 ) )
-    except:
+    if not self.ServerIsRunning():
       return False
 
-
-  def ServerTerminated( self ):
-    """ Check if the server process has already terminated. """
-    return ( self._omnisharp_phandle is not None and
-             self._omnisharp_phandle.poll() is not None )
+    try:
+      return self._GetResponse( '/checkreadystatus', timeout = .2 )
+    except requests.exceptions.RequestException:
+      return False
 
 
   def _SolutionFile( self ):

ycmd/completers/javascript/tern_completer.py

@@ -440,8 +440,7 @@ def _StopServerNoLock( self ):
 
 
   def _ServerIsRunning( self ):
-    return ( self._server_handle is not None and
-             self._server_handle.poll() is None )
+    return utils.ProcessIsRunning( self._server_handle )
 
 
   def _GetType( self, request_data ):

ycmd/handlers.py

@@ -131,7 +131,7 @@ def GetHealthy():
   _logger.info( 'Received health request' )
   if request.query.include_subservers:
     cs_completer = _server_state.GetFiletypeCompleter( ['cs'] )
-    return _JsonResponse( cs_completer.ServerIsRunning() )
+    return _JsonResponse( cs_completer.ServerIsHealthy() )
   return _JsonResponse( True )
 
 

ycmd/tests/cs/cs_handlers_test.py

@@ -57,11 +57,11 @@ def _WaitUntilOmniSharpServerReady( self, filename ):
         success = True
         break
       request = self._BuildRequest( completer_target = 'filetype_default',
-                                    command_arguments = [ 'ServerTerminated' ],
+                                    command_arguments = [ 'ServerIsRunning' ],
                                     filepath = filename,
                                     filetype = 'cs' )
       result = self._app.post_json( '/run_completer_command', request ).json
-      if result:
+      if not result:
         raise RuntimeError( "OmniSharp failed during startup." )
       time.sleep( 0.2 )
       retries = retries - 1

ycmd/utils.py

@@ -171,6 +171,10 @@ def OnTravis():
   return 'TRAVIS' in os.environ
 
 
+def ProcessIsRunning( handle ):
+  return handle is not None and handle.poll() is None
+
+
 # From here: http://stackoverflow.com/a/8536476/1672783
 def TerminateProcess( pid ):
   if OnWindows():