UserLock: Fix multi-threaded access to a global variable

This commit is contained in:
Eelco Dolstra 2017-01-25 13:00:38 +01:00
parent a55f589720
commit 951357e5fb
No known key found for this signature in database
GPG key ID: 8170B4726D7198DE

View file

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