commit cebc727502f4966771be68acdd7bd1c4c633902e
author David James <davidjames@chromium.org> Wed Jul 17 16:45:05 2013 -0700
committer ChromeBot <chrome-bot@google.com> Thu Jul 18 15:54:00 2013 -0700
tree ff40e348c8e4b4301b3b9e394aa19bc2c811e992
parent 273e78e3eca0ffcd6de5d0883c54e226216401b7

Don't try to clean up the tee process.

We don't want the critical section to try to clean up the tee process,
so we run Tee (forked off) outside of it. This prevents a deadlock
because the Tee process only exits when its pipe is closed, and the
critical section accidentally holds on to that file handle.

BUG=chromium:261402
TEST=Test on buildbot and verify hang is gone.
TEST=Paladin trybot run.

Change-Id: Ia457bbc9fe8c60de6b5c7474b5f490776a2cac41
Reviewed-on: https://gerrit.chromium.org/gerrit/62427
Commit-Queue: David James <davidjames@chromium.org>
Reviewed-by: David James <davidjames@chromium.org>
Tested-by: David James <davidjames@chromium.org>

scripts/cbuildbot.py

diff --git a/scripts/cbuildbot.py b/scripts/cbuildbot.py
index 5a7d100..5b4a230 100644
--- a/scripts/cbuildbot.py
+++ b/scripts/cbuildbot.py
@@ -1401,6 +1401,18 @@
     _BackupPreviousLog(log_file)
 
   with cros_build_lib.ContextManagerStack() as stack:
+    # TODO(ferringb): update this once https://gerrit.chromium.org/gerrit/25359
+    # is landed- it's sensitive to the manifest-versions cache path.
+    options.preserve_paths = set(['manifest-versions', '.cache',
+                                  'manifest-versions-internal'])
+    if log_file is not None:
+      # We don't want the critical section to try to clean up the tee process,
+      # so we run Tee (forked off) outside of it. This prevents a deadlock
+      # because the Tee process only exits when its pipe is closed, and the
+      # critical section accidentally holds on to that file handle.
+      stack.Add(tee.Tee, log_file)
+      options.preserve_paths.add(_DEFAULT_LOG_DIR)
+
     critical_section = stack.Add(cleanup.EnforcedCleanupSection)
     stack.Add(sudo.SudoKeepAlive)
 
@@ -1410,14 +1422,6 @@
       stack.Add(osutils.TempDir, prefix='cbuildbot-tmp', set_global=True)
       logging.debug("Cbuildbot tempdir is %r.", os.environ.get('TMP'))
 
-    # TODO(ferringb): update this once https://gerrit.chromium.org/gerrit/25359
-    # is landed- it's sensitive to the manifest-versions cache path.
-    options.preserve_paths = set(['manifest-versions', '.cache',
-                                  'manifest-versions-internal'])
-    if log_file is not None:
-      stack.Add(tee.Tee, log_file)
-      options.preserve_paths.add(_DEFAULT_LOG_DIR)
-
     if options.cgroups:
       stack.Add(cgroups.SimpleContainChildren, 'cbuildbot')