Fix deadlock

This commit is contained in:
Sebastian Messmer 2018-12-26 15:12:01 +01:00
parent 648263d062
commit 2bae1281c6
2 changed files with 35 additions and 23 deletions

View File

@ -27,18 +27,18 @@ namespace fsblobstore {
constexpr fspp::num_bytes_t DirBlob::DIR_LSTAT_SIZE;
DirBlob::DirBlob(unique_ref<Blob> blob, std::function<fspp::num_bytes_t (const blockstore::BlockId&)> getLstatSize) :
FsBlob(std::move(blob)), _getLstatSize(getLstatSize), _entries(), _mutex(), _changed(false) {
FsBlob(std::move(blob)), _getLstatSize(getLstatSize), _getLstatSizeMutex(), _entries(), _entriesAndChangedMutex(), _changed(false) {
ASSERT(baseBlob().blobType() == FsBlobView::BlobType::DIR, "Loaded blob is not a directory");
_readEntriesFromBlob();
}
DirBlob::~DirBlob() {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_writeEntriesToBlob();
}
void DirBlob::flush() {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_writeEntriesToBlob();
baseBlob().flush();
}
@ -64,17 +64,17 @@ void DirBlob::_readEntriesFromBlob() {
}
void DirBlob::AddChildDir(const std::string &name, const BlockId &blobId, fspp::mode_t mode, fspp::uid_t uid, fspp::gid_t gid, timespec lastAccessTime, timespec lastModificationTime) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_addChild(name, blobId, fspp::Dir::EntryType::DIR, mode, uid, gid, lastAccessTime, lastModificationTime);
}
void DirBlob::AddChildFile(const std::string &name, const BlockId &blobId, fspp::mode_t mode, fspp::uid_t uid, fspp::gid_t gid, timespec lastAccessTime, timespec lastModificationTime) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_addChild(name, blobId, fspp::Dir::EntryType::FILE, mode, uid, gid, lastAccessTime, lastModificationTime);
}
void DirBlob::AddChildSymlink(const std::string &name, const blockstore::BlockId &blobId, fspp::uid_t uid, fspp::gid_t gid, timespec lastAccessTime, timespec lastModificationTime) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
auto mode = fspp::mode_t().addSymlinkFlag()
.addUserReadFlag().addUserWriteFlag().addUserExecFlag()
.addGroupReadFlag().addGroupWriteFlag().addGroupExecFlag()
@ -91,41 +91,41 @@ void DirBlob::_addChild(const std::string &name, const BlockId &blobId,
void DirBlob::AddOrOverwriteChild(const std::string &name, const BlockId &blobId, fspp::Dir::EntryType entryType,
fspp::mode_t mode, fspp::uid_t uid, fspp::gid_t gid, timespec lastAccessTime, timespec lastModificationTime,
std::function<void (const blockstore::BlockId &blockId)> onOverwritten) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.addOrOverwrite(name, blobId, entryType, mode, uid, gid, lastAccessTime, lastModificationTime, onOverwritten);
_changed = true;
}
void DirBlob::RenameChild(const blockstore::BlockId &blockId, const std::string &newName, std::function<void (const blockstore::BlockId &blockId)> onOverwritten) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.rename(blockId, newName, onOverwritten);
_changed = true;
}
boost::optional<const DirEntry&> DirBlob::GetChild(const string &name) const {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
return _entries.get(name);
}
boost::optional<const DirEntry&> DirBlob::GetChild(const BlockId &blockId) const {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
return _entries.get(blockId);
}
void DirBlob::RemoveChild(const string &name) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.remove(name);
_changed = true;
}
void DirBlob::RemoveChild(const BlockId &blockId) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.remove(blockId);
_changed = true;
}
void DirBlob::AppendChildrenTo(vector<fspp::Dir::Entry> *result) const {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
result->reserve(result->size() + _entries.size());
for (const auto &entry : _entries) {
result->emplace_back(entry.type(), entry.name());
@ -137,7 +137,18 @@ fspp::num_bytes_t DirBlob::lstat_size() const {
}
fspp::Node::stat_info DirBlob::statChild(const BlockId &blockId) const {
return statChildWithKnownSize(blockId, _getLstatSize(blockId));
std::unique_lock<std::mutex> lock(_getLstatSizeMutex);
auto lstatSizeGetter = _getLstatSize;
// The following unlock is important to avoid deadlock.
// ParallelAccessFsBlobStore::load() causes a call to DirBlob::setLstatSizeGetter,
// so their lock ordering first locks the ParallelAccessStore::_mutex, then the DirBlob::_getLstatSizeMutex.
// this requires us to free DirBlob::_getLstatSizeMutex before calling into lstatSizeGetter(), because
// lstatSizeGetter can call ParallelAccessFsBlobStore::load().
lock.unlock();
auto lstatSize = lstatSizeGetter(blockId);
return statChildWithKnownSize(blockId, lstatSize);
}
fspp::Node::stat_info DirBlob::statChildWithKnownSize(const BlockId &blockId, fspp::num_bytes_t size) const {
@ -163,44 +174,44 @@ fspp::Node::stat_info DirBlob::statChildWithKnownSize(const BlockId &blockId, fs
}
void DirBlob::updateAccessTimestampForChild(const BlockId &blockId, TimestampUpdateBehavior timestampUpdateBehavior) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
if (_entries.updateAccessTimestampForChild(blockId, timestampUpdateBehavior)) {
_changed = true;
}
}
void DirBlob::updateModificationTimestampForChild(const BlockId &blockId) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.updateModificationTimestampForChild(blockId);
_changed = true;
}
void DirBlob::chmodChild(const BlockId &blockId, fspp::mode_t mode) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.setMode(blockId, mode);
_changed = true;
}
void DirBlob::chownChild(const BlockId &blockId, fspp::uid_t uid, fspp::gid_t gid) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
if(_entries.setUidGid(blockId, uid, gid)) {
_changed = true;
}
}
void DirBlob::utimensChild(const BlockId &blockId, timespec lastAccessTime, timespec lastModificationTime) {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_entries.setAccessTimes(blockId, lastAccessTime, lastModificationTime);
_changed = true;
}
void DirBlob::setLstatSizeGetter(std::function<fspp::num_bytes_t(const blockstore::BlockId&)> getLstatSize) {
std::unique_lock<std::mutex> lock(_mutex);
_getLstatSize = getLstatSize;
std::lock_guard<std::mutex> lock(_getLstatSizeMutex);
_getLstatSize = std::move(getLstatSize);
}
cpputils::unique_ref<blobstore::Blob> DirBlob::releaseBaseBlob() {
std::unique_lock<std::mutex> lock(_mutex);
std::unique_lock<std::mutex> lock(_entriesAndChangedMutex);
_writeEntriesToBlob();
return FsBlob::releaseBaseBlob();
}

View File

@ -83,8 +83,9 @@ namespace cryfs {
cpputils::unique_ref<blobstore::Blob> releaseBaseBlob() override;
std::function<fspp::num_bytes_t (const blockstore::BlockId&)> _getLstatSize;
mutable std::mutex _getLstatSizeMutex;
DirEntryList _entries;
mutable std::mutex _mutex;
mutable std::mutex _entriesAndChangedMutex;
bool _changed;
DISALLOW_COPY_AND_ASSIGN(DirBlob);