Merge pull request #860 from aslater/master

Revised fix for ycmd terminate race condition

python/ycm/server/ycmd.py

@@ -28,9 +28,9 @@ import waitress
 import signal
 from ycm import user_options_store
 from ycm import extra_conf_store
+from ycm import utils
 from ycm.server.watchdog_plugin import WatchdogPlugin
 
-
 def YcmCoreSanityCheck():
   if 'ycm_core' in sys.modules:
     raise RuntimeError( 'ycm_core already imported, ycmd has a bug!' )
@@ -38,8 +38,25 @@ def YcmCoreSanityCheck():
 
 # We manually call sys.exit() on SIGTERM and SIGINT so that atexit handlers are
 # properly executed.
-def SetUpSignalHandler():
+def SetUpSignalHandler(stdout, stderr, keep_logfiles):
   def SignalHandler( signum, frame ):
+    if stderr:
+      # Reset stderr, just in case something tries to use it
+      tmp = sys.stderr
+      sys.stderr = sys.__stderr__
+      tmp.close()
+    if stdout:
+      # Reset stdout, just in case something tries to use it
+      tmp = sys.stdout
+      sys.stdout = sys.__stdout__
+      tmp.close()
+
+    if not keep_logfiles:
+      if stderr:
+        utils.RemoveIfExists( stderr )
+      if stdout:
+        utils.RemoveIfExists( stdout )
+
     sys.exit()
 
   for sig in [ signal.SIGTERM,
@@ -61,8 +78,19 @@ def Main():
                        help = 'num idle seconds before server shuts down')
   parser.add_argument( '--options_file', type = str, default = '',
                        help = 'file with user options, in JSON format' )
+  parser.add_argument( '--stdout', type = str, default = None,
+                       help = 'optional file to use for stdout' )
+  parser.add_argument( '--stderr', type = str, default = None,
+                       help = 'optional file to use for stderr' )
+  parser.add_argument( '--keep_logfiles', action = 'store_true', default = None,
+                       help = 'retain logfiles after the server exits' )
   args = parser.parse_args()
 
+  if args.stdout is not None:
+    sys.stdout = open(args.stdout, "w")
+  if args.stderr is not None:
+    sys.stderr = open(args.stderr, "w")
+
   numeric_level = getattr( logging, args.log.upper(), None )
   if not isinstance( numeric_level, int ):
     raise ValueError( 'Invalid log level: %s' % args.log )
@@ -86,7 +114,7 @@ def Main():
   # preload has executed.
   from ycm.server import handlers
   handlers.UpdateUserOptions( options )
-  SetUpSignalHandler()
+  SetUpSignalHandler(args.stdout, args.stderr, args.keep_logfiles)
   handlers.app.install( WatchdogPlugin( args.idle_suicide_seconds ) )
   waitress.serve( handlers.app,
                   host = args.host,

python/ycm/youcompleteme.py

@@ -22,6 +22,7 @@ import vim
 import tempfile
 import json
 import signal
+from subprocess import PIPE
 from ycm import vimsupport
 from ycm import utils
 from ycm.diagnostic_interface import DiagnosticInterface
@@ -84,26 +85,22 @@ class YouCompleteMe( object ):
     self._SetupServer()
     self._ycmd_keepalive.Start()
 
-
   def _SetupServer( self ):
     server_port = utils.GetUnusedLocalhostPort()
     with tempfile.NamedTemporaryFile( delete = False ) as options_file:
       self._temp_options_filename = options_file.name
       json.dump( dict( self._user_options ), options_file )
       options_file.flush()
+
       args = [ utils.PathToPythonInterpreter(),
                _PathToServerScript(),
                '--port={0}'.format( server_port ),
                '--options_file={0}'.format( options_file.name ),
                '--log={0}'.format( self._user_options[ 'server_log_level' ] ),
                '--idle_suicide_seconds={0}'.format(
-                  SERVER_IDLE_SUICIDE_SECONDS ) ]
+                  SERVER_IDLE_SUICIDE_SECONDS )]
 
-      BaseRequest.server_location = 'http://localhost:' + str( server_port )
-
-      if self._user_options[ 'server_use_vim_stdout' ]:
-        self._server_popen = utils.SafePopen( args )
-      else:
+      if not self._user_options[ 'server_use_vim_stdout' ]:
         filename_format = os.path.join( utils.PathToTempDir(),
                                         'server_{port}_{std}.log' )
 
@@ -111,15 +108,18 @@ class YouCompleteMe( object ):
                                                       std = 'stdout' )
         self._server_stderr = filename_format.format( port = server_port,
                                                       std = 'stderr' )
+        args.append('--stdout={0}'.format( self._server_stdout ))
+        args.append('--stderr={0}'.format( self._server_stderr ))
+
+        if self._user_options[ 'server_keep_logfiles' ]:
+          args.append('--keep_logfiles')
+
+      self._server_popen = utils.SafePopen( args, stdout = PIPE, stderr = PIPE)
+
+      BaseRequest.server_location = 'http://localhost:' + str( server_port )
 
-        with open( self._server_stderr, 'w' ) as fstderr:
-          with open( self._server_stdout, 'w' ) as fstdout:
-            self._server_popen = utils.SafePopen( args,
-                                                  stdout = fstdout,
-                                                  stderr = fstderr )
     self._NotifyUserIfServerCrashed()
 
-
   def _IsServerAlive( self ):
     returncode = self._server_popen.poll()
     # When the process hasn't finished yet, poll() returns None.
@@ -151,13 +151,6 @@ class YouCompleteMe( object ):
       self._server_popen.terminate()
     utils.RemoveIfExists( self._temp_options_filename )
 
-    if not self._user_options[ 'server_keep_logfiles' ]:
-      if self._server_stderr:
-        utils.RemoveIfExists( self._server_stderr )
-      if self._server_stdout:
-        utils.RemoveIfExists( self._server_stdout )
-
-
   def RestartServer( self ):
     vimsupport.PostVimMessage( 'Restarting ycmd server...' )
     self._user_notified_about_crash = False