From d3f9e24f91aaedb56cedec5608efd27917a48eab Mon Sep 17 00:00:00 2001 From: Vladimir Panteleev Date: Sat, 28 Dec 2013 05:02:43 +0000 Subject: [PATCH] std.file: Fix race condition when mkdirRecurse runs concurrently mkdirRecurse would crash if a directory was created between its exists() and mkdir() calls. This could occur if mkdirRecurse was called at the same time from mutiple threads or processes. Instead of relying on the exists() check to indicate whether the next mkdir() should succeed, we explicitly ignore "already exists" errors from the OS. Note that this changes the behavior of mkdirRecurse when the leaf directory already existed: previously it would throw, whereas now it will silently succeed. The new behavior is conformant with some other implementations of "recursive mkdir": the -p flag of the GNU tool, Ruby's mkpath, Java's mkdirs, and Perl's make_path. (On the other hand, the old behavior was equivalent to the behavior of Python's makedirs). --- std/file.d | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/std/file.d b/std/file.d index 5121dee44..aa2d5e140 100644 --- a/std/file.d +++ b/std/file.d @@ -1366,6 +1366,27 @@ void mkdir(in char[] pathname) } } +// Same as mkdir but ignores "already exists" errors. +// Returns: "true" if the directory was created, +// "false" if it already existed. +private bool ensureDirExists(in char[] pathname) +{ + version(Windows) + { + if (CreateDirectoryW(std.utf.toUTF16z(pathname), null)) + return true; + cenforce(GetLastError() == ERROR_ALREADY_EXISTS, pathname.idup); + } + else version(Posix) + { + if (core.sys.posix.sys.stat.mkdir(toStringz(pathname), octal!777) == 0) + return true; + cenforce(errno == EEXIST, pathname); + } + enforce(pathname.isDir, new FileException(pathname.idup)); + return false; +} + /**************************************************** * Make directory and all parent directories as needed. * @@ -1388,12 +1409,30 @@ void mkdirRecurse(in char[] pathname) } if (!baseName(pathname).empty) { - mkdir(pathname); + ensureDirExists(pathname); } } unittest { + { + immutable basepath = deleteme ~ "_dir"; + scope(exit) rmdirRecurse(basepath); + + auto path = buildPath(basepath, "a", "..", "b"); + mkdirRecurse(path); + path = path.buildNormalizedPath; + assert(path.isDir); + + path = buildPath(basepath, "c"); + write(path, ""); + assertThrown!FileException(mkdirRecurse(path)); + + path = buildPath(basepath, "d"); + mkdirRecurse(path); + mkdirRecurse(path); // should not throw + } + // bug3570 { immutable basepath = deleteme ~ "_dir";