fix #674 - cache needs to be locked for concurrent dub runs

- implement lockFile/tryLockFile using mkdir
- use lockFile to synchronize cache and
  package download

source/dub/dub.d

@@ -472,14 +472,14 @@ class Dub {
 	{
 		import std.stdio;
 		import std.ascii : newline;
-		
+
 		// Split comma-separated lists
 		string[] requestedDataSplit =
 			requestedData
 			.map!(a => a.splitter(",").map!strip)
 			.joiner()
 			.array();
-		
+
 		auto data = m_project.listBuildSettings(platform, config, buildType,
 			requestedDataSplit, formattingCompiler, nullDelim);
 
@@ -561,20 +561,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.
@@ -1107,4 +1117,3 @@ class DependencyVersionResolver : DependencyResolver!(Dependency, Dependency) {
 		return null;
 	}
 }
-

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);
 	}
 }
 
@@ -111,18 +146,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;
@@ -274,7 +309,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 {
@@ -284,7 +319,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"]) );

source/dub/packagesupplier.d

@@ -155,13 +155,14 @@ class RegistryPackageSupplier : PackageSupplier {
 
 	void cacheOp(Path cacheDir, CacheOp op)
 	{
+		if (!cacheDir.existsFile())
+			mkdirRecurse(cacheDir.toNativeString());
 		auto path = cacheDir ~ cacheFileName;
+		auto lock = lockFile(path.toNativeString() ~ ".lock", 1.seconds);
 		final switch (op)
 		{
 		case CacheOp.store:
 			if (!m_metadataCacheDirty) return;
-			if (!cacheDir.existsFile())
-				mkdirRecurse(cacheDir.toNativeString());
 			// TODO: method is slow due to Json escaping
 			writeJsonFile(path, m_metadataCache.serializeToJson());
 			break;

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