diff --git a/source/dub/dub.d b/source/dub/dub.d index 9b487df4c..c6f6a1635 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -575,20 +575,30 @@ class Dub { if (m_dryRun) return null; logDiagnostic("Acquiring package zip file"); - auto dload = m_projectPath ~ ".dub/temp/downloads"; - auto tempfname = packageId ~ "-" ~ (ver.startsWith('~') ? ver[1 .. $] : ver) ~ ".zip"; - auto tempFile = m_tempPath ~ tempfname; - string sTempFile = tempFile.toNativeString(); - if (exists(sTempFile)) std.file.remove(sTempFile); - supplier.retrievePackage(tempFile, packageId, dep, (options & FetchOptions.usePrerelease) != 0); // Q: continue on fail? - scope(exit) std.file.remove(sTempFile); - - logInfo("Placing %s %s to %s...", packageId, ver, placement.toNativeString()); + auto clean_package_version = ver[ver.startsWith("~") ? 1 : 0 .. $]; clean_package_version = clean_package_version.replace("+", "_"); // + has special meaning for Optlink + if (!placement.existsFile()) + mkdirRecurse(placement.toNativeString()); Path dstpath = placement ~ (packageId ~ "-" ~ clean_package_version); - return m_packageManager.storeFetchedPackage(tempFile, pinfo, dstpath); + if (auto lock = tryLockFile(dstpath.toNativeString() ~ ".lock")) // avoid concurrent fetch + { + auto path = getTempFile(packageId, ".zip"); + supplier.retrievePackage(path, packageId, dep, (options & FetchOptions.usePrerelease) != 0); // Q: continue on fail? + scope(exit) std.file.remove(path.toNativeString()); + + logInfo("Placing %s %s to %s...", packageId, ver, placement.toNativeString()); + + return m_packageManager.storeFetchedPackage(path, pinfo, dstpath); + } + else + { + logInfo("Waiting for concurrent dub to fetch %s %s.", packageId, ver); + lockFile(dstpath.toNativeString() ~ ".lock", 30.seconds); // wait for other dub instance + m_packageManager.refresh(false); + return m_packageManager.getPackage(packageId, ver, dstpath); + } } /// Removes a given package from the list of present/cached modules. diff --git a/source/dub/internal/utils.d b/source/dub/internal/utils.d index bf257454d..6b1038487 100644 --- a/source/dub/internal/utils.d +++ b/source/dub/internal/utils.d @@ -14,6 +14,7 @@ import dub.internal.vibecompat.inet.url; import dub.version_; // todo: cleanup imports. +import core.thread; import std.algorithm : startsWith; import std.array; import std.conv; @@ -27,13 +28,13 @@ import std.zip; version(DubUseCurl) import std.net.curl; +private Path[] temporary_files; + Path getTempDir() { return Path(std.file.tempDir()); } -private Path[] temporary_files; - Path getTempFile(string prefix, string extension = null) { import std.uuid : randomUUID; @@ -43,11 +44,45 @@ Path getTempFile(string prefix, string extension = null) return path; } +// lockfile based on atomic mkdir +struct LockFile +{ + bool opCast(T:bool)() { return !!path; } + ~this() { if (path) rmdir(path); } + string path; +} + +auto tryLockFile(string path) +{ + import std.file; + if (collectException(mkdir(path))) + return LockFile(null); + return LockFile(path); +} + +auto lockFile(string path, Duration wait) +{ + import std.datetime, std.file; + auto t0 = Clock.currTime(); + auto dur = 1.msecs; + while (true) + { + if (!collectException(mkdir(path))) + return LockFile(path); + enforce(Clock.currTime() - t0 < wait, "Failed to lock '"~path~"'."); + if (dur < 1024.msecs) // exponentially increase sleep time + dur *= 2; + Thread.sleep(dur); + } +} + static ~this() { foreach (path; temporary_files) { - std.file.remove(path.toNativeString()); + auto spath = path.toNativeString(); + if (spath.exists) + std.file.remove(spath); } } @@ -127,18 +162,18 @@ void runCommands(in string[] commands, string[string] env = null) version(Windows) enum nullFile = "NUL"; else version(Posix) enum nullFile = "/dev/null"; else static assert(0); - + auto childStdout = stdout; auto childStderr = stderr; auto config = Config.retainStdout | Config.retainStderr; - + // Disable child's stdout/stderr depending on LogLevel auto logLevel = getLogLevel(); if(logLevel >= LogLevel.warn) childStdout = File(nullFile, "w"); if(logLevel >= LogLevel.none) childStderr = File(nullFile, "w"); - + foreach(cmd; commands){ logDiagnostic("Running %s", cmd); Pid pid; @@ -290,7 +325,7 @@ auto fuzzySearch(R)(R strings, string input){ /** If T is a bitfield-style enum, this function returns a string range listing the names of all members included in the given value. - + Example: --------- enum Bits { @@ -300,7 +335,7 @@ auto fuzzySearch(R)(R strings, string input){ c = 1<<2, a_c = a | c, } - + assert( bitFieldNames(Bits.none).equals(["none"]) ); assert( bitFieldNames(Bits.a).equals(["a"]) ); assert( bitFieldNames(Bits.a_c).equals(["a", "c", "a_c"]) ); diff --git a/source/dub/packagesupplier.d b/source/dub/packagesupplier.d index 64096c32e..9d952724a 100644 --- a/source/dub/packagesupplier.d +++ b/source/dub/packagesupplier.d @@ -170,7 +170,7 @@ class RegistryPackageSupplier : PackageSupplier { if (!cacheDir.existsFile()) mkdirRecurse(cacheDir.toNativeString()); // TODO: method is slow due to Json escaping - writeJsonFile(path, m_metadataCache.serializeToJson()); + atomicWriteJsonFile(path, m_metadataCache.serializeToJson()); break; case CacheOp.load: diff --git a/test/issue674-concurrent-dub.sh b/test/issue674-concurrent-dub.sh new file mode 100755 index 000000000..b31417ed0 --- /dev/null +++ b/test/issue674-concurrent-dub.sh @@ -0,0 +1,21 @@ +#!/bin/bash + +set -e -o pipefail + +TMPDIR=$(mktemp -d) +echo ${TMPDIR} + +function cleanup { + rm -rf ${TMPDIR} +} +trap cleanup EXIT + +cd ${TMPDIR} && $DUB fetch --cache=local bloom & +pid1=$! +cd ${TMPDIR} && $DUB fetch --cache=local bloom & +pid2=$! +wait $pid1 +wait $pid2 +if [ ! -d ${TMPDIR}/bloom* ]; then + exit 1 +fi