diff options
author | Eelco Dolstra <edolstra@gmail.com> | 2017-01-25T12·00+0100 |
---|---|---|
committer | Eelco Dolstra <edolstra@gmail.com> | 2017-01-26T19·40+0100 |
commit | 951357e5fb4cd0804e729866f204b635add926a3 (patch) | |
tree | e0c80f7bc7332c82e8647c42f3364d8cb819f7e6 /src/libstore/build.cc | |
parent | a55f589720e6499ed8ca1e3dd63ae18c52782150 (diff) |
UserLock: Fix multi-threaded access to a global variable
Diffstat (limited to '')
-rw-r--r-- | src/libstore/build.cc | 70 |
1 files changed, 40 insertions, 30 deletions
diff --git a/src/libstore/build.cc b/src/libstore/build.cc index e0859269dce4..6250de13cbb2 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); } |