Fix rare logdir races by adding "_logdirfilecount" to the 'PID' when...

Fix rare logdir races by adding "_logdirfilecount" to the 'PID' when generating a unique logdir path
TBR=bruening
Review URL: http://codereview.chromium.org/8816017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113191 0039d316-1c4b-4281-b951-d872f2087c98

tools/valgrind/browser_wrapper_win.py

@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import glob
 import os
 import re
 import sys
@@ -11,7 +12,6 @@ import subprocess
 # support layout_tests, remove Dr. Memory specific code and verify it works
 # on a "clean" Mac.
 
-wrapper_pid = os.getpid()
 testcase_name = None
 for arg in sys.argv:
   m = re.match("\-\-test\-name=(.*)", arg)
@@ -29,11 +29,20 @@ cmd_to_run = sys.argv[1:]
 # http://code.google.com/p/drmemory/issues/detail?id=684 fixed.
 logdir_idx = cmd_to_run.index("-logdir")
 old_logdir = cmd_to_run[logdir_idx + 1]
-cmd_to_run[logdir_idx + 1] += "\\testcase.%d.logs" % wrapper_pid
+
+wrapper_pid = str(os.getpid())
+
+# On Windows, there is a chance of PID collision. We avoid it by appending the
+# number of entries in the logdir at the end of wrapper_pid.
+# This number is monotonic and we can't have two simultaneously running wrappers
+# with the same PID.
+wrapper_pid += "_%d" % len(glob.glob(old_logdir + "\\*"))
+
+cmd_to_run[logdir_idx + 1] += "\\testcase.%s.logs" % wrapper_pid
 os.makedirs(cmd_to_run[logdir_idx + 1])
 
 if testcase_name:
-  f = open(old_logdir + "\\testcase.%d.name" % wrapper_pid, "w")
+  f = open(old_logdir + "\\testcase.%s.name" % wrapper_pid, "w")
   print >>f, testcase_name
   f.close()
 

tools/valgrind/valgrind_test.py

@@ -921,28 +921,29 @@ class DrMemory(BaseTool):
     else:
       testcases = glob.glob(self.log_dir + "/testcase.*.logs")
       # If we have browser wrapper, the per-test logdirs are named as
-      # "testcase.wrapper_PID".
-      # Let's extract the list of wrapper_PIDs and name it ppids
-      ppids = set([int(f.split(".")[-2]) for f in testcases])
+      # "testcase.wrapper_PID.name".
+      # Let's extract the list of wrapper_PIDs and name it ppids.
+      # NOTE: ppids may contain '_', i.e. they are not ints!
+      ppids = set([f.split(".")[-2] for f in testcases])
 
       for ppid in ppids:
         testcase_name = None
         try:
-          f = open(self.log_dir + ("/testcase.%d.name" % ppid))
+          f = open("%s/testcase.%s.name" % (self.log_dir, ppid))
           testcase_name = f.read().strip()
           f.close()
         except IOError:
           pass
         print "====================================================="
-        print " Below is the report for drmemory wrapper PID=%d." % ppid
+        print " Below is the report for drmemory wrapper PID=%s." % ppid
         if testcase_name:
           print " It was used while running the `%s` test." % testcase_name
         else:
           # TODO(timurrrr): hm, the PID line is suppressed on Windows...
           print " You can find the corresponding test"
-          print " by searching the above log for 'PID=%d'" % ppid
+          print " by searching the above log for 'PID=%s'" % ppid
         sys.stdout.flush()
-        ppid_filenames = glob.glob("%s/testcase.%d.logs/*/results.txt" %
+        ppid_filenames = glob.glob("%s/testcase.%s.logs/*/results.txt" %
                                    (self.log_dir, ppid))
         ret |= analyzer.Report(ppid_filenames, testcase_name, False)
         print "====================================================="