From 412b902b0edeef4f02f6b3548227fc2d25084abb Mon Sep 17 00:00:00 2001 From: David James Date: Fri, 15 Jul 2011 19:54:16 -0700 Subject: [PATCH] Fix race condition when starting env_syncer in enter_chroot.sh. Sometimes, two processes might start the env_syncer at the same time. If this occurs, then the pid file will get overwritten and one of the processes won't be killed. I've also fixed the env_syncer to properly daemonize, such that cbuildbot won't wait on the env_syncer to be killed before exiting. This requires closing stdin, stderr, and stdout. BUG=chromium-os:17680 TEST=Reproduce hang, and verify that this fixes the hang. Verify that multiple chroots can be entered at the same time and that the env_syncer is killed when the last chroot exits. Verify that cbuildbot is not kept hanging even if the env_syncer doesn't exit. Change-Id: I3cb9fbdf47a406e818e12cf91593b8e3481aa578 Reviewed-on: http://gerrit.chromium.org/gerrit/4232 Reviewed-by: Ryan Cui Tested-by: David James --- enter_chroot.sh | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/enter_chroot.sh b/enter_chroot.sh index 8fa593a6ea..79e2f49627 100755 --- a/enter_chroot.sh +++ b/enter_chroot.sh @@ -136,6 +136,10 @@ function env_sync_proc { sudo chown ${USER} ${FLAGS_chroot}/${file} 1>&2 done + # Drop stdin, stderr, stdout, and chroot lock. + # This is needed for properly daemonizing the process. + exec 0>&- 1>&- 2>&- 200>&- + while true; do # Sync files for file in ${sync_files}; do @@ -186,19 +190,20 @@ function setup_env { # Don't use sudo -v since that has issues on machines w/ no password. sudo echo "" > /dev/null - # Syncer proc, but only the first time - if ! [ -f "${SYNCERPIDFILE}" ] || \ - ! [ -d /proc/$(cat "${SYNCERPIDFILE}") ]; then - debug "Starting sync process" - env_sync_proc & - echo $! > "${SYNCERPIDFILE}" - disown $! - fi - ( flock 200 echo $$ >> "$LOCKFILE" + # If there isn't a syncer daemon started already, start one. This daemon + # is killed by the enter_chroot that exits last. + if ! [ -f "${SYNCERPIDFILE}" ] || \ + ! [ -d /proc/$(cat "${SYNCERPIDFILE}") ]; then + debug "Starting sync process" + env_sync_proc & + echo $! > "${SYNCERPIDFILE}" + disown $! + fi + debug "Mounting chroot environment." ensure_mounted none "-t proc" /proc ensure_mounted none "-t sysfs" /sys