From 7c2e05b4304c29e589617e46e903c76d35b11f33 Mon Sep 17 00:00:00 2001 From: Dan Jacques Date: Thu, 6 Jul 2017 10:03:30 -0700 Subject: [PATCH] [cipd] Fix CIPD bootstraps run concurrently. It's currently possible for CIPD bootstraps that provision concurrently to: 1) On Linux, step on each other during download, and 2) On Windows, fail. Fix these respective scripts so that bootstraps are safe to use concurrently. On Linux and Mac, we download to a temporary file and use "mv" (atomic) to write it to the final destination. Concurrent initializations will perform parallel downloads, execute the "mv", and copy their downloaded file to the destination path. On Windows, we use filesystem locking to lock the operation and ensure that only one download can happen. BUG=chromium:739195 TEST=local - Ran in parallel on Windows, Linux, and Max. Change-Id: Ie050d37598da67389f21728e781bd58904ef9c17 Reviewed-on: https://chromium-review.googlesource.com/560521 Reviewed-by: Robbie Iannucci Commit-Queue: Daniel Jacques --- cipd | 16 ++++++++++++++-- cipd.ps1 | 37 ++++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/cipd b/cipd index b299157891..fc8de1a9ef 100755 --- a/cipd +++ b/cipd @@ -52,8 +52,20 @@ if [ ! -e "$CLIENT" ]; then echo "Bootstrapping cipd client for ${PLAT}-${ARCH}..." echo "From $URL" if hash curl 2> /dev/null ; then - curl "$URL" -f -A "$USER_AGENT" -L -o "$CLIENT" - chmod +x "$CLIENT" + # Download the client into a temporary file, then move it into the final + # location atomically. + # + # This wonky tempdir method works on Linux and Mac. + CIPD_CLIENT_TMP=$(\ + mktemp -p "$MYPATH" 2>/dev/null || \ + mktemp "$MYPATH/.cipd_client.XXXXXXX") + + curl "$URL" -f -A "$USER_AGENT" -L -o "$CIPD_CLIENT_TMP" + chmod +x "$CIPD_CLIENT_TMP" + + set +e + mv "$CIPD_CLIENT_TMP" "$CLIENT" + set -e else echo Your platform is missing the \`curl\` command. Please use your package echo manager to install it before continuing. diff --git a/cipd.ps1 b/cipd.ps1 index 973af4f1f8..32cbace241 100644 --- a/cipd.ps1 +++ b/cipd.ps1 @@ -40,15 +40,34 @@ try { } $Env:CIPD_HTTP_USER_AGENT_PREFIX = $user_agent -if (!(Test-Path $client)) { - echo "Bootstrapping cipd client for $plat-$arch..." - echo "From $url" - # TODO(iannucci): It would be really nice if there was a way to get this to - # show progress without also completely destroying the download speed, but - # I can't seem to find a way to do it. Patches welcome :) - $wc = (New-Object System.Net.WebClient) - $wc.Headers.add('User-Agent', $user_agent) - $wc.DownloadFile($url, $client) + +# Use a lock fle to prevent simultaneous processes from stepping on each other. +$cipd_lock = Join-Path $myPath -ChildPath '.cipd_client.lock' +while ($true) { + $cipd_lock_file = $false + try { + $cipd_lock_file = [IO.File]::OpenWrite($cipd_lock) + + if (!(Test-Path $client)) { + echo "Bootstrapping cipd client for $plat-$arch..." + echo "From $url" + + # TODO(iannucci): It would be really nice if there was a way to get this to + # show progress without also completely destroying the download speed, but + # I can't seem to find a way to do it. Patches welcome :) + $wc = (New-Object System.Net.WebClient) + $wc.Headers.add('User-Agent', $user_agent) + $wc.DownloadFile($url, $client) + } + break + } catch { + echo "CIPD lock is held, trying again after delay..." + Start-Sleep -s 1 + } finally { + if ($cipd_lock_file) { + $cipd_lock_file.close() + } + } } $_ = & $client selfupdate -version "$cipdClientVer"