Skip to content

Commit

Permalink
Fix race condition in MakeDirs (bazelbuild#644)
Browse files Browse the repository at this point in the history
If there are 2 concurrent processes creating the same directories, then it will fail

I noticed this on the CI errors of bazelbuild#567, and a co-worker was hitting it
today. I suspect invalidating the worker or other updates triggered big
rebuilds.

I don't know of a perfect repro but, at a 10% rate: invoking Bazel to
build 2 swift libraries in the same BUILD file.
  • Loading branch information
jerrymarino authored Jun 10, 2021
1 parent f9629ff commit d02b2cb
Showing 1 changed file with 34 additions and 2 deletions.
36 changes: 34 additions & 2 deletions tools/common/file_system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
#include <sys/types.h>
#include <unistd.h>

#include <iostream>
#include <string>
#include <string.h>

#ifdef __APPLE__
#include <copyfile.h>
Expand Down Expand Up @@ -99,7 +101,13 @@ bool MakeDirs(const std::string &path, int mode) {
struct stat dir_stats;
if (stat(path.c_str(), &dir_stats) == 0) {
// Return true if the directory already exists.
return S_ISDIR(dir_stats.st_mode);
if (!S_ISDIR(dir_stats.st_mode)) {
std::cerr << "error: path exists and isn't a directory "
<< strerror(errno) << " (" << path.c_str() << ") \n";
return false;
} else {
return true;
}
}

// Recurse to create the parent directory.
Expand All @@ -108,5 +116,29 @@ bool MakeDirs(const std::string &path, int mode) {
}

// Create the directory that was requested.
return mkdir(path.c_str(), mode) == 0;
int mkdir_ret = mkdir(path.c_str(), mode);
if (mkdir_ret == 0) {
return true;
}

// Save the mkdir errno for better reporting.
int mkdir_errno = errno;

// Deal with a race condition when 2 `MakeDirs` are running at the same time:
// one `mkdir` invocation will fail in each recursive call at different
// points. Don't recurse here to avoid an infinite loop in failure cases.
if (errno == EEXIST && stat(path.c_str(), &dir_stats) == 0) {
if (!S_ISDIR(dir_stats.st_mode)) {
std::cerr << "error: path exists and isn't a directory "
<< strerror(errno) << " (" << path.c_str() << ") \n";
return false;
} else {
// Return true if the directory already exists.
return true;
}
}

std::cerr << "error:" << strerror(mkdir_errno) << " (" << path.c_str()
<< ") \n";
return false;
}

0 comments on commit d02b2cb

Please sign in to comment.