server: add an error feedback mechanism for when the daemon fails to launch

There's a recurring problem on Windows where `hg serve -d` will randomly fail to
spawn a detached process.  The reason for the failure is completely hidden, and
it takes hours to get a single failure on my laptop.  All this does is redirect
stdout/stderr of the child to a file until the lock file is freed, and then the
parent dumps it out if it fails to spawn.

I chose to put the output into the lock file because that is always cleaned up.
There's no way to report errors after that anyway.  On Windows, killdaemons.py
is roughly `kill -9`, so this ensures that junk won't pile up.

This may end up being a case of EADDRINUSE.  At least that's what I saw spit out
a few times (among other odd errors and missing output on Windows).  But I also
managed to get the same thing on Fedora 26 by running test-hgwebdir.t with
--loop -j10 for several hours.  Running `netstat` immediately after killing that
run printed a wall of sockets in the TIME_WAIT state, which were gone a couple
seconds later.  I couldn't match up ports that failed, because --loop doesn't
print out the message about the port that was used.  So maybe the fix is to
rotate the use of HGPORT[12] in the tests.  But, let's collect some more data
first.

mercurial/server.py

@@ -1,174 +1,207 b''
 # server.py - utility and factory of server
 #
 # Copyright 2005-2007 Matt Mackall <[e-mail]>
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
 from __future__ import absolute_import
 
 import os
 import tempfile
 
 from .i18n import _
 
 from . import (
     chgserver,
     cmdutil,
     commandserver,
     error,
     hgweb,
     pycompat,
     util,
 )
 
 from .utils import (
     procutil,
 )
 
 def runservice(opts, parentfn=None, initfn=None, runfn=None, logfile=None,
                runargs=None, appendpid=False):
     '''Run a command as a service.'''
 
+    # When daemonized on Windows, redirect stdout/stderr to the lockfile (which
+    # gets cleaned up after the child is up and running), so that the parent can
+    # read and print the error if this child dies early.  See 594dd384803c.  On
+    # other platforms, the child can write to the parent's stdio directly, until
+    # it is redirected prior to runfn().
+    if pycompat.iswindows and opts['daemon_postexec']:
+        for inst in opts['daemon_postexec']:
+            if inst.startswith('unlink:'):
+                lockpath = inst[7:]
+                if os.path.exists(lockpath):
+                    procutil.stdout.flush()
+                    procutil.stderr.flush()
+
+                    fd = os.open(lockpath,
+                                 os.O_WRONLY | os.O_APPEND | os.O_BINARY)
+                    try:
+                        os.dup2(fd, 1)
+                        os.dup2(fd, 2)
+                    finally:
+                        os.close(fd)
+
     def writepid(pid):
         if opts['pid_file']:
             if appendpid:
                 mode = 'ab'
             else:
                 mode = 'wb'
             fp = open(opts['pid_file'], mode)
             fp.write('%d\n' % pid)
             fp.close()
 
     if opts['daemon'] and not opts['daemon_postexec']:
         # Signal child process startup with file removal
         lockfd, lockpath = tempfile.mkstemp(prefix='hg-service-')
         os.close(lockfd)
         try:
             if not runargs:
                 runargs = procutil.hgcmd() + pycompat.sysargv[1:]
             runargs.append('--daemon-postexec=unlink:%s' % lockpath)
             # Don't pass --cwd to the child process, because we've already
             # changed directory.
             for i in xrange(1, len(runargs)):
                 if runargs[i].startswith('--cwd='):
                     del runargs[i]
                     break
                 elif runargs[i].startswith('--cwd'):
                     del runargs[i:i + 2]
                     break
             def condfn():
                 return not os.path.exists(lockpath)
             pid = procutil.rundetached(runargs, condfn)
             if pid < 0:
+                # If the daemonized process managed to write out an error msg,
+                # report it.
+                if pycompat.iswindows and os.path.exists(lockpath):
+                    with open(lockpath) as log:
+                        for line in log:
+                            procutil.stderr.write(line)
                 raise error.Abort(_('child process failed to start'))
             writepid(pid)
         finally:
             util.tryunlink(lockpath)
         if parentfn:
             return parentfn(pid)
         else:
             return
 
     if initfn:
         initfn()
 
     if not opts['daemon']:
         writepid(procutil.getpid())
 
     if opts['daemon_postexec']:
         try:
             os.setsid()
         except AttributeError:
             pass
+
+        lockpath = None
         for inst in opts['daemon_postexec']:
             if inst.startswith('unlink:'):
                 lockpath = inst[7:]
-                os.unlink(lockpath)
             elif inst.startswith('chdir:'):
                 os.chdir(inst[6:])
             elif inst != 'none':
                 raise error.Abort(_('invalid value for --daemon-postexec: %s')
                                   % inst)
         procutil.hidewindow()
         procutil.stdout.flush()
         procutil.stderr.flush()
 
         nullfd = os.open(os.devnull, os.O_RDWR)
         logfilefd = nullfd
         if logfile:
             logfilefd = os.open(logfile, os.O_RDWR | os.O_CREAT | os.O_APPEND,
                                 0o666)
         os.dup2(nullfd, 0)
         os.dup2(logfilefd, 1)
         os.dup2(logfilefd, 2)
         if nullfd not in (0, 1, 2):
             os.close(nullfd)
         if logfile and logfilefd not in (0, 1, 2):
             os.close(logfilefd)
 
+        # Only unlink after redirecting stdout/stderr, so Windows doesn't
+        # complain about a sharing violation.
+        if lockpath:
+            os.unlink(lockpath)
+
     if runfn:
         return runfn()
 
 _cmdservicemap = {
     'chgunix': chgserver.chgunixservice,
     'pipe': commandserver.pipeservice,
     'unix': commandserver.unixforkingservice,
 }
 
 def _createcmdservice(ui, repo, opts):
     mode = opts['cmdserver']
     try:
         return _cmdservicemap[mode](ui, repo, opts)
     except KeyError:
         raise error.Abort(_('unknown mode %s') % mode)
 
 def _createhgwebservice(ui, repo, opts):
     # this way we can check if something was given in the command-line
     if opts.get('port'):
         opts['port'] = util.getport(opts.get('port'))
 
     alluis = {ui}
     if repo:
         baseui = repo.baseui
         alluis.update([repo.baseui, repo.ui])
     else:
         baseui = ui
     webconf = opts.get('web_conf') or opts.get('webdir_conf')
     if webconf:
         if opts.get('subrepos'):
             raise error.Abort(_('--web-conf cannot be used with --subrepos'))
 
         # load server settings (e.g. web.port) to "copied" ui, which allows
         # hgwebdir to reload webconf cleanly
         servui = ui.copy()
         servui.readconfig(webconf, sections=['web'])
         alluis.add(servui)
     elif opts.get('subrepos'):
         servui = ui
 
         # If repo is None, hgweb.createapp() already raises a proper abort
         # message as long as webconf is None.
         if repo:
             webconf = dict()
             cmdutil.addwebdirpath(repo, "", webconf)
     else:
         servui = ui
 
     optlist = ("name templates style address port prefix ipv6"
                " accesslog errorlog certificate encoding")
     for o in optlist.split():
         val = opts.get(o, '')
         if val in (None, ''): # should check against default options instead
             continue
         for u in alluis:
             u.setconfig("web", o, val, 'serve')
 
     app = hgweb.createapp(baseui, repo, webconf)
     return hgweb.httpservice(servui, app, opts)
 
 def createservice(ui, repo, opts):
     if opts["cmdserver"]:
         return _createcmdservice(ui, repo, opts)
     else:
         return _createhgwebservice(ui, repo, opts)