UserLock: Fix multi-threaded access to a global variable

1 files changed, 40 insertions, 30 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index e0859269d..6250de13c 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -434,7 +434,7 @@ private:
        close that file again (without closing the original file
        descriptor), we lose the lock.  So we have to be *very* careful
        not to open a lock file on which we are holding a lock. */
-    static PathSet lockedPaths; /* !!! not thread-safe */
+    static Sync<PathSet> lockedPaths_;
 
     Path fnUserLock;
     AutoCloseFD fdUserLock;
@@ -460,7 +460,7 @@ public:
 };
 
 
-PathSet UserLock::lockedPaths;
+Sync<PathSet> UserLock::lockedPaths_;
 
 
 UserLock::UserLock()
@@ -499,39 +499,48 @@ UserLock::UserLock()
 
         fnUserLock = (format("%1%/userpool/%2%") % settings.nixStateDir % pw->pw_uid).str();
 
-        if (lockedPaths.find(fnUserLock) != lockedPaths.end())
-            /* We already have a lock on this one. */
-            continue;
+        {
+            auto lockedPaths(lockedPaths_.lock());
+            if (lockedPaths->count(fnUserLock))
+                /* We already have a lock on this one. */
+                continue;
+            lockedPaths->insert(fnUserLock);
+        }
+
+        try {
 
-        AutoCloseFD fd = open(fnUserLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600);
-        if (!fd)
-            throw SysError(format("opening user lock ‘%1%’") % fnUserLock);
+            AutoCloseFD fd = open(fnUserLock.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0600);
+            if (!fd)
+                throw SysError(format("opening user lock ‘%1%’") % fnUserLock);
 
-        if (lockFile(fd.get(), ltWrite, false)) {
-            fdUserLock = std::move(fd);
-            lockedPaths.insert(fnUserLock);
-            user = i;
-            uid = pw->pw_uid;
+            if (lockFile(fd.get(), ltWrite, false)) {
+                fdUserLock = std::move(fd);
+                user = i;
+                uid = pw->pw_uid;
 
-            /* Sanity check... */
-            if (uid == getuid() || uid == geteuid())
-                throw Error(format("the Nix user should not be a member of ‘%1%’")
-                    % settings.buildUsersGroup);
+                /* Sanity check... */
+                if (uid == getuid() || uid == geteuid())
+                    throw Error(format("the Nix user should not be a member of ‘%1%’")
+                        % settings.buildUsersGroup);
 
 #if __linux__
-            /* Get the list of supplementary groups of this build user.  This
-               is usually either empty or contains a group such as "kvm".  */
-            supplementaryGIDs.resize(10);
-            int ngroups = supplementaryGIDs.size();
-            int err = getgrouplist(pw->pw_name, pw->pw_gid,
-                supplementaryGIDs.data(), &ngroups);
-            if (err == -1)
-                throw Error(format("failed to get list of supplementary groups for ‘%1%’") % pw->pw_name);
-
-            supplementaryGIDs.resize(ngroups);
+                /* Get the list of supplementary groups of this build user.  This
+                   is usually either empty or contains a group such as "kvm".  */
+                supplementaryGIDs.resize(10);
+                int ngroups = supplementaryGIDs.size();
+                int err = getgrouplist(pw->pw_name, pw->pw_gid,
+                    supplementaryGIDs.data(), &ngroups);
+                if (err == -1)
+                    throw Error(format("failed to get list of supplementary groups for ‘%1%’") % pw->pw_name);
+
+                supplementaryGIDs.resize(ngroups);
 #endif
 
-            return;
+                return;
+            }
+
+        } catch (...) {
+            lockedPaths_.lock()->erase(fnUserLock);
         }
     }
 
@@ -543,8 +552,9 @@ UserLock::UserLock()
 
 UserLock::~UserLock()
 {
-    assert(lockedPaths.count(fnUserLock));
-    lockedPaths.erase(fnUserLock);
+    auto lockedPaths(lockedPaths_.lock());
+    assert(lockedPaths->count(fnUserLock));
+    lockedPaths->erase(fnUserLock);
 }