Patchwork [3,of,4] cmdutil.service: move pidfile writing to the parent in daemon mode

login
register
mail settings
Submitter Siddharth Agarwal
Date Oct. 2, 2013, 11:19 p.m.
Message ID <567a2b3f19214d776ca2.1380755985@dev1091.prn1.facebook.com>
Download mbox | patch
Permalink /patch/2715/
State Accepted
Commit 0532c8f8e911d374a08b71ae1426ec0ef664dab3
Headers show

Comments

Siddharth Agarwal - Oct. 2, 2013, 11:19 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1380752449 25200
#      Wed Oct 02 15:20:49 2013 -0700
# Node ID 567a2b3f19214d776ca20ac0e44b206b75e6580c
# Parent  e4b7774aec7e068677e811ddf7ddd24ed9ce3d65
cmdutil.service: move pidfile writing to the parent in daemon mode

There is a potential race here, which I suspect I've spotted in the wild, where
something reads the pid file after the parent exits but before the child has
had a chance to write to it. Moving writing the file to the parent causes this
to no longer be an issue.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -497,6 +497,7 @@ 
             pid = util.rundetached(runargs, condfn)
             if pid < 0:
                 raise util.Abort(_('child process failed to start'))
+            writepid(pid)
         finally:
             try:
                 os.unlink(lockpath)
@@ -511,7 +512,8 @@ 
     if initfn:
         initfn()
 
-    writepid(os.getpid())
+    if not opts['daemon']:
+        writepid(os.getpid())
 
     if opts['daemon_pipefds']:
         lockpath = opts['daemon_pipefds']