facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2025-01-04 03:06:30 +03:00
Code Issues Projects Releases Wiki Activity

always load affected inodes in rename(), and update lock ordering

Browse Source 
Summary:
This updates the TreeInode::rename() code to handle concurrency better.
In particular:

- The code now ensures that both the source inode being renamed and destination
  inode (if it exists) are loaded.  This simplifies issues when an inode is
  being loaded at the same time a rename is in progress.  This ensures that any
  pending load is processed before the rename takes place.  (All promises for
  the load might not be fulfilled before the rename completes, but the relevant
  TreeInode and InodeMap data structures are updated before the rename occurs.)

  This does mean that the rename code potentially might have to retry several
  times if the inode it began loading is no longer the affected source or
  destination or child once the load completes.  However, this seems like a
  reasonable trade-off, compared to dealing with the complications that would
  arise with the load code having to handle renames occuring before load
  completion.

- The code now implements proper lock ordering, to avoid acquiring locks in
  conflicting orders that might cause deadlock with other threads also trying
  to acquire the same locks.  The InodeLocks.md document has been updated to
  clarify the TreeInode lock ordering requirements.

Reviewed By: wez

Differential Revision: D4493526

fbshipit-source-id: 627393fafad90eb551aea62be7762d59ed043abe
This commit is contained in:
Adam Simpkins 2017-02-03 18:34:27 -08:00 committed by Facebook Github Bot
parent 8884b46b3f
commit 8b4c984b28
8 changed files with 1318 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
eden/docs/InodeLocks.md
View File

@ -42,12 +42,42 @@ lock.
- A FileInode's lock may be acquired while holding its parent TreeInode's
`contents_` lock.
- In some situations the same thread acquires multiple `contents_` locks
together. These must be acquired in the following order:
- If TreeInode A is a parent or ancestor of TreeInode B, A's `contents_` lock
must be acquired first.
- If TreeInodes A and B are not in a parent/child relationship, acquire A's
`contents_` lock first if and only if A's address is lower than B's
In some situations the same thread acquires multiple `contents_` locks
together.
- Some code paths hold a parent TreeInode's `contents_` lock while accessing
its children, and then acquire a child TreeInode's `contents_` lock while
still holding the parent TreeInode's lock.
- The `rename()` code may hold up to 3 TreeInode locks. It always holds the
`contents_` lock on both the source TreeInode and the destination
TreeInode. Additionally, if the destination name refers to an existing
TreeInode, the rename() holds its `contents_` lock as well, to ensure that
it is empty, and to prevent new entries from being created inside this
directory once the rename starts.
To prevent deadlocks, the lock ordering constraints for TreeInode `contents_`
are as follows:
- If you are not holding the mountpoint rename lock, you can only acquire
a TreeInode `contents_` lock if the other `contents_` locks you are holding
are for this TreeInode's immediate parents. (e.g., if you are already
holding another `contents_` lock it must be for this TreeInode's parent. If
you are holding two other `contents_` locks it must be for this TreeInode's
parent and grandparent.)
Note however that acquiring multiple TreeInode contents locks discouraged.
When possible it is preferred to release the lock on the parent TreeInode
before locking the child. Acquiring locks on more than 2 levels of the tree
hierarchy is technically safe from a lock ordering perspective, but is also
strongly discouraged.
- If you are holding the mountpoint rename lock, it is safe to acquire multiple
TreeInode locks at a time. However, if there is an ancestor/child
relationship between any of the TreeInode's, the ancestor lock must be
acquired first. This avoids lock ordering issues with other threads that are
not holding the rename lock. Among unrelated TreeInodes no particular
ordering is required.
## EdenMount's rename lock:

64
eden/docs/Rename.md Normal file
View File

@ -0,0 +1,64 @@
A few notes about renames:
# Rename Lock
There is a mountpoint-wide rename lock that is held during any rename or unlink
operation. An Inode's path cannot be changed without holding this lock.
However, we currently do not hold the rename lock when creating new files or
directories. Therefore TreeEntry `contents_.entries` fields may change even
when the rename lock is not held. (We could potentially revisit this choice
later and require holding the rename lock even when creating new inodes.)
# Renaming over directories
Rename supports renaming one directory over an existing directory, as long as
the destination directory is empty. This means we must (a) be able to safely
check if the directory is currently empty, and (b) be able to prevent new files
or directories from being created inside the destination directory once the
rename has started.
We currently achieve this by acquiring the destination directory's `contents_`
lock. This does mean that a rename operation may hold up to 3 TreeInode locks
concurrently: the source directory, the destination parent directory, and the
destination child directory. The [InodeLocks](InodeLocks.md) document
describes the lock ordering requirements for acquiring these 3 locks.
This also means that create() and mkdir() operations must check if the parent
directory is unlinked *after* acquiring the parent directory's contents lock.
# Handling unloaded children
When rename() (and unlink()/rmdir()) is invoked, the parent directories have
already been loaded (typically having been identified via inode number).
However, the affected children may not have been loaded yet, and are referred
to by name.
We had a few choices for how to deal with this situation.
For now we have opted to always load the child entries in question before
performing the rename. This is slightly tricky, as loading the child may take
some time, and another rename or unlink operation may also be in progress, and
may affect the child in question before our operation can take place. One
option would have been to hold the rename lock while waiting on the children to
be loaded. However, this would have blocked all other rename/unlink/rmdir
operations for the duration of the load, which seems undesirable. Instead, we
wait for the load to complete, then double check to confirm that the named
entry that we desire is actually loaded. The original inode we loaded may have
been renamed or unlinked, so we may find an unloaded entry or no entry at all.
If we find an unloaded entry we have to repeat the load operation. We
therefore may have to retry loading the requested children multiple times
before we can make progress, but we should eventually succeed or fail. Once
the children are loaded the rename itself is then fairly straightforward.
Another option would have been to allow the rename even though the requested
children are not loaded. The main downside with this approach is that we still
need to confirm if the destination child is an empty directory or not. This
would have meant either loading the destination child inode anyway, or storing
some extra data to track if an unloaded inode is an empty directory or not.
This also makes the inode loading code more complicated, as an inode may be
unlinked or renamed while it is already in the process of being loaded. When
the load completes we would need to double-check which parent TreeInode the new
entry needs to be inserted into. All-in-all this felt more complicated than
simply always loading the affected children before performing
rename/unlink/rmdir operations.

1
eden/fs/inodes/EdenMount.h
View File

@ -266,6 +266,7 @@ class EdenMount {
*/
class RenameLock : public std::unique_lock<folly::SharedMutex> {
public:
RenameLock() {}
explicit RenameLock(EdenMount* mount)
: std::unique_lock<folly::SharedMutex>{mount->renameMutex_} {}

456
eden/fs/inodes/TreeInode.cpp
View File

@ -34,6 +34,7 @@
using folly::Future;
using folly::makeFuture;
using folly::StringPiece;
using folly::Unit;
using std::make_unique;
using std::unique_ptr;
@ -677,8 +678,6 @@ folly::Future<folly::Unit> TreeInode::unlink(PathComponentPiece name) {
}
folly::Future<folly::Unit> TreeInode::rmdir(PathComponentPiece name) {
// TODO: We should probably grab the mountpoint-wide rename lock here,
// and hold it during the child lookup. This would avoid having to retry.
return getOrLoadChildTree(name).then(
[ self = inodePtrFromThis(),
childName = PathComponent{name} ](const TreeInodePtr& child) {
@ -789,56 +788,254 @@ folly::Future<folly::Unit> TreeInode::rmdirImpl(
return folly::Unit{};
}
std::unique_ptr<InodeBase> TreeInode::renameHelper(
Dir* sourceContents,
PathComponentPiece sourceName,
TreeInodePtr destParent,
Dir* destContents,
PathComponentPiece destName,
const RenameLock& renameLock) {
auto sourceEntIter = sourceContents->entries.find(sourceName);
if (sourceEntIter == sourceContents->entries.end()) {
throw InodeError(ENOENT, inodePtrFromThis(), sourceName);
/**
* A helper class that stores all locks required to perform a rename.
*
* This class helps acquire the locks in the correct order.
*/
struct TreeInode::TreeRenameLocks {
TreeRenameLocks() {}
void acquireLocks(
TreeInode* srcTree,
TreeInode* destTree,
PathComponentPiece destName);
void reset() {
*this = TreeRenameLocks();
}
auto destEntIter = destContents->entries.find(destName);
const RenameLock& renameLock() const {
return renameLock_;
}
if (mode_to_dtype(sourceEntIter->second->mode) == dtype_t::Dir &&
destEntIter != destContents->entries.end()) {
// When renaming a directory, the destination must either not exist or
// it must be an empty directory
if (mode_to_dtype(destEntIter->second->mode) != dtype_t::Dir) {
throw InodeError(ENOTDIR, destParent, destName);
Dir* srcContents() {
return srcContents_;
}
Dir* destContents() {
return destContents_;
}
const PathMap<std::unique_ptr<Entry>>::iterator& destChildIter() const {
return destChildIter_;
}
InodeBase* destChild() const {
DCHECK(destChildExists());
return destChildIter_->second->inode;
}
bool destChildExists() const {
return destChildIter_ != destContents_->entries.end();
}
bool destChildIsDirectory() const {
DCHECK(destChildExists());
return mode_to_dtype(destChildIter_->second->mode) == dtype_t::Dir;
}
bool destChildIsEmpty() const {
DCHECK_NOTNULL(destChildContents_);
return destChildContents_->entries.empty();
}
private:
void lockDestChild(PathComponentPiece destName);
/**
* The mountpoint-wide rename lock.
*/
RenameLock renameLock_;
/**
* Locks for the contents of the source and destination directories.
* If the source and destination directories are the same, only
* srcContentsLock_ is set. However, srcContents_ and destContents_ above are
* always both set, so that destContents_ can be used regardless of wether
* the source and destination are both the same directory or not.
*/
folly::Synchronized<Dir>::LockedPtr srcContentsLock_;
folly::Synchronized<Dir>::LockedPtr destContentsLock_;
folly::Synchronized<Dir>::LockedPtr destChildContentsLock_;
/**
* Pointers to the source and destination directory contents.
*
* These may both point to the same contents when the source and destination
* directory are the same.
*/
Dir* srcContents_{nullptr};
Dir* destContents_{nullptr};
Dir* destChildContents_{nullptr};
/**
* An iterator pointing to the destination child entry in
* destContents_->entries.
* This may point to destContents_->entries.end() if the destination child
* does not exist.
*/
PathMap<std::unique_ptr<Entry>>::iterator destChildIter_;
};
Future<Unit> TreeInode::rename(
PathComponentPiece name,
TreeInodePtr destParent,
PathComponentPiece destName) {
materializeDirAndParents();
if (destParent.get() != this) {
destParent->materializeDirAndParents();
}
bool needSrc = false;
bool needDest = false;
{
// Acquire the locks required to do the rename
TreeRenameLocks locks;
locks.acquireLocks(this, destParent.get(), destName);
// Look up the source entry. The destination entry info was already
// loaded by TreeRenameLocks::acquireLocks().
auto srcIter = locks.srcContents()->entries.find(name);
if (srcIter == locks.srcContents()->entries.end()) {
// The source path does not exist. Fail the rename.
return makeFuture<Unit>(InodeError(ENOENT, inodePtrFromThis(), name));
}
Entry* srcEntry = srcIter->second.get();
// If the directory is loaded, check to see if it contains anything
if (destEntIter->second->inode != nullptr) {
auto destDir = dynamic_cast<TreeInode*>(destEntIter->second->inode);
if (!destDir) {
throw InodeError(
EIO,
destParent,
destName,
"inconsistency between contents and inodes objects");
}
// Perform as much input validation as possible now, before starting inode
// loads that might be necessary.
if (!destDir->contents_.rlock()->entries.empty()) {
throw InodeError(ENOTEMPTY, destParent, destName);
// Validate invalid file/directory replacement
if (mode_to_dtype(srcEntry->mode) == dtype_t::Dir) {
// The source is a directory.
// The destination must not exist, or must be an empty directory,
// or the exact same directory.
if (locks.destChildExists()) {
if (!locks.destChildIsDirectory()) {
VLOG(4) << "attempted to rename directory " << getLogPath() << "/"
<< name << " over file " << destParent->getLogPath() << "/"
<< destName;
return makeFuture<Unit>(InodeError(ENOTDIR, destParent, destName));
} else if (
locks.destChild() != srcEntry->inode && !locks.destChildIsEmpty()) {
VLOG(4) << "attempted to rename directory " << getLogPath() << "/"
<< name << " over non-empty directory "
<< destParent->getLogPath() << "/" << destName;
return makeFuture<Unit>(InodeError(ENOTEMPTY, destParent, destName));
}
}
} else {
// This directory is not currently loaded.
// This means that it cannot be materialized, so it only contains the
// contents from its Tree. We don't ever track empty Trees, so therefore
// the directory cannot be empty.
throw InodeError(ENOTEMPTY, destParent, destName);
// The source is not a directory.
// The destination must not exist, or must not be a directory.
if (locks.destChildExists() && locks.destChildIsDirectory()) {
VLOG(4) << "attempted to rename file " << getLogPath() << "/" << name
<< " over directory " << destParent->getLogPath() << "/"
<< destName;
return makeFuture<Unit>(InodeError(EISDIR, destParent, destName));
}
}
// Make sure the destination directory is not unlinked.
if (destParent->isUnlinked()) {
VLOG(4) << "attempted to rename file " << getLogPath() << "/" << name
<< " into deleted directory " << destParent->getLogPath()
<< " ( as " << destName << ")";
return makeFuture<Unit>(InodeError(ENOENT, destParent));
}
// Check to see if we need to load the source or destination inodes
needSrc = !srcEntry->inode;
needDest = locks.destChildExists() && !locks.destChild();
// If we don't have to load anything now, we can immediately perform the
// rename.
if (!needSrc && !needDest) {
return doRename(std::move(locks), name, srcIter, destParent, destName);
}
// If we are still here we have to load either the source or destination,
// or both. Release the locks before we try loading them.
//
// (We could refactor getOrLoadChild() a little bit so that we could start
// the loads with the locks still held, rather than releasing them just for
// getOrLoadChild() to re-acquire them temporarily. This isn't terribly
// important for now, though.)
}
// Once we finish the loads, we have to re-run all the rename() logic.
// Other renames or unlinks may have occurred in the meantime, so all of the
// validation above has to be redone.
auto onLoadFinished = [
self = inodePtrFromThis(),
nameCopy = name.copy(),
destParent,
destNameCopy = destName.copy()
]() {
return self->rename(nameCopy, destParent, destNameCopy);
};
if (needSrc && needDest) {
auto srcFuture = getOrLoadChild(name);
auto destFuture = destParent->getOrLoadChild(destName);
return folly::collect(srcFuture, destFuture).then(onLoadFinished);
} else if (needSrc) {
return getOrLoadChild(name).then(onLoadFinished);
} else {
CHECK(needDest);
return destParent->getOrLoadChild(destName).then(onLoadFinished);
}
}
namespace {
bool isAncestor(const RenameLock& renameLock, TreeInode* a, TreeInode* b) {
auto parent = b->getParent(renameLock);
while (parent) {
if (parent.get() == a) {
return true;
}
parent = parent->getParent(renameLock);
}
return false;
}
}
Future<Unit> TreeInode::doRename(
TreeRenameLocks&& locks,
PathComponentPiece srcName,
PathMap<std::unique_ptr<Entry>>::iterator srcIter,
TreeInodePtr destParent,
PathComponentPiece destName) {
Entry* srcEntry = srcIter->second.get();
// If the source and destination refer to exactly the same file,
// then just succeed immediately. Nothing needs to be done in this case.
if (locks.destChildExists() && srcEntry->inode == locks.destChild()) {
return folly::Unit{};
}
// If we are doing a directory rename, sanity check that the destination
// directory is not a child of the source directory. The Linux kernel
// generally should avoid invoking FUSE APIs with an invalid rename like
// this, but we want to check in case rename() gets invoked via some other
// non-FUSE mechanism.
//
// We don't have to worry about the source being a child of the destination
// directory. That will have already been caught by the earlier check that
// ensures the destination directory is non-empty.
if (mode_to_dtype(srcEntry->mode) == dtype_t::Dir) {
// Our caller has already verified that the source is also a
// directory here.
auto* srcTreeInode =
boost::polymorphic_downcast<TreeInode*>(srcEntry->inode);
if (srcTreeInode == destParent.get() ||
isAncestor(locks.renameLock(), srcTreeInode, destParent.get())) {
return makeFuture<Unit>(InodeError(EINVAL, destParent, destName));
}
}
// If we haven't actually materialized it yet, the rename() call will
// fail. So don't try that.
if (sourceEntIter->second->materialized) {
if (srcEntry->materialized) {
auto contentDir = getOverlay()->getContentDir();
auto absoluteSourcePath = contentDir + getPathBuggy() + sourceName;
auto absoluteSourcePath = contentDir + getPathBuggy() + srcName;
auto absoluteDestPath = contentDir + destParent->getPathBuggy() + destName;
folly::checkUnixError(
::rename(absoluteSourcePath.c_str(), absoluteDestPath.c_str()),
@ -852,96 +1049,125 @@ std::unique_ptr<InodeBase> TreeInode::renameHelper(
// Success.
// Update the destination with the source data (this copies in the hash if
// it happens to be set).
auto& destEnt = destContents->entries[destName];
// Note: sourceEntIter may have been invalidated by the line above in the
// case that the source and destination dirs are the same. We need to
// recompute that iterator now to be safe.
sourceEntIter = sourceContents->entries.find(sourceName);
std::unique_ptr<InodeBase> deletedInode;
// FIXME: If destEnt exists but destEnt->inode is not loaded, we need to tell
// the InodeMap so it can update its state if the child is present in the
// unloadedInodes_ map.
if (destEnt && destEnt->inode) {
deletedInode =
destEnt->inode->markUnlinked(destParent.get(), destName, renameLock);
auto* childInode = srcEntry->inode;
if (locks.destChildExists()) {
deletedInode = locks.destChild()->markUnlinked(
destParent.get(), destName, locks.renameLock());
// Replace the destination contents entry with the source data
locks.destChildIter()->second = std::move(srcIter->second);
} else {
auto ret = locks.destContents()->entries.emplace(
destName, std::move(srcIter->second));
CHECK(ret.second);
// If the source and destination directory are the same, then inserting the
// destination entry may have invalidated our source entry iterator, so we
// have to look it up again.
if (destParent.get() == this) {
srcIter = locks.srcContents()->entries.find(srcName);
}
}
// We want to move in the data from the source.
destEnt = std::move(sourceEntIter->second);
// FIXME: If our child is not loaded, we need to tell the InodeMap so it
// can update its state if the child is present in the unloadedInodes_ map.
if (destEnt->inode) {
destEnt->inode->updateLocation(destParent, destName, renameLock);
}
// Inform the child inode that it has been moved
childInode->updateLocation(destParent, destName, locks.renameLock());
// Now remove the source information
sourceContents->entries.erase(sourceEntIter);
return deletedInode;
locks.srcContents()->entries.erase(srcIter);
// Save the overlay data
const auto& overlay = getOverlay();
overlay->saveOverlayDir(getPathBuggy(), locks.srcContents());
if (destParent.get() != this) {
// We have already verified that destParent is not unlinked, and we are
// holding the rename lock which prevents it from being renamed or unlinked
// while we are operating, so getPath() must have a value here.
overlay->saveOverlayDir(
destParent->getPath().value(), locks.destContents());
}
// Release the rename locks before we destroy the deleted destination child
// inode (if it exists).
locks.reset();
deletedInode.reset();
return folly::Unit{};
}
folly::Future<folly::Unit> TreeInode::rename(
PathComponentPiece name,
TreeInodePtr newParent,
PathComponentPiece newName) {
// Grab a mountpoint-wide rename lock so that no other rename
// operations can happen while we are running.
auto renameLock = getMount()->acquireRenameLock();
/**
* Acquire the locks necessary for a rename operation.
*
* We acquire multiple locks here:
* A) Mountpoint rename lock
* B) Source directory contents_ lock
* C) Destination directory contents_ lock
* E) Destination child contents_ (assuming the destination name
* refers to an existing directory).
*
* This function ensures the locks are held with the proper ordering.
* Since we hold the rename lock first, we can acquire multiple TreeInode
* contents_ locks at once, but we must still ensure that we acquire locks on
* ancestor TreeInode's before any of their descendants.
*/
void TreeInode::TreeRenameLocks::acquireLocks(
TreeInode* srcTree,
TreeInode* destTree,
PathComponentPiece destName) {
// First grab the mountpoint-wide rename lock.
renameLock_ = srcTree->getMount()->acquireRenameLock();
auto myPath = getPathBuggy();
auto destDirPath = newParent->getPathBuggy();
// Check pre-conditions with a read lock before we materialize anything
// in case we're processing spurious rename for a non-existent entry;
// we don't want to materialize part of a tree if we're not actually
// going to do any work in it.
// There are some more complex pre-conditions that we'd like to check
// before materializing, but we cannot do so in a race free manner
// without locking each of the associated objects. The existence
// check is sufficient to avoid the majority of the potentially
// wasted effort.
contents_.withRLock([&](const auto& contents) {
auto entIter = contents.entries.find(name);
if (entIter == contents.entries.end()) {
throw InodeError(ENOENT, this->inodePtrFromThis(), name);
}
});
materializeDirAndParents();
// Can't use SYNCHRONIZED_DUAL for both cases, as we'd self-deadlock by trying
// to wlock the same thing twice
std::unique_ptr<InodeBase> deletedInode;
if (newParent.get() == this) {
contents_.withWLock([&](auto& contents) {
deletedInode = this->renameHelper(
&contents, name, newParent, &contents, newName, renameLock);
this->getOverlay()->saveOverlayDir(myPath, &contents);
});
if (srcTree == destTree) {
// If the source and destination directories are the same,
// then there is really only one parent directory to lock.
srcContentsLock_ = srcTree->contents_.wlock();
srcContents_ = &*srcContentsLock_;
destContents_ = &*srcContentsLock_;
// Look up the destination child entry, and lock it if is is a directory
lockDestChild(destName);
} else if (isAncestor(renameLock_, srcTree, destTree)) {
// If srcTree is an ancestor of destTree, we must acquire the lock on
// srcTree first.
srcContentsLock_ = srcTree->contents_.wlock();
srcContents_ = &*srcContentsLock_;
destContentsLock_ = destTree->contents_.wlock();
destContents_ = &*destContentsLock_;
lockDestChild(destName);
} else {
newParent->materializeDirAndParents();
// In all other cases, lock destTree and destChild before srcTree,
// as long as we verify that destChild and srcTree are not the same.
//
// It is not possible for srcTree to be an ancestor of destChild,
// since we have confirmed that srcTree is not destTree nor an ancestor of
// destTree.
destContentsLock_ = destTree->contents_.wlock();
destContents_ = &*destContentsLock_;
lockDestChild(destName);
// TODO: SYNCHRONIZED_DUAL is not the correct locking order to use here.
// We need to figure out if the source and dest are ancestors/children of
// each other. If so we have to lock the ancestor first. Otherwise we may
// deadlock with other operations that always acquire parent directory
// locks first (e.g., rmdir())
SYNCHRONIZED_DUAL(
sourceContents, contents_, destContents, newParent->contents_) {
deletedInode = renameHelper(
&sourceContents, name, newParent, &destContents, newName, renameLock);
getOverlay()->saveOverlayDir(myPath, &sourceContents);
getOverlay()->saveOverlayDir(destDirPath, &destContents);
// While srcTree cannot be an ancestor of destChild, it might be the
// same inode. Don't try to lock the same TreeInode twice in this case.
//
// The rename will be failed later since this must be an error, but for now
// we keep going and let the exact error be determined later.
// This will either be ENOENT (src entry doesn't exist) or ENOTEMPTY
// (destChild is not empty since the src entry exists).
if (destChildExists() && destChild() == srcTree) {
CHECK_NOTNULL(destChildContents_);
srcContents_ = destChildContents_;
} else {
srcContentsLock_ = srcTree->contents_.wlock();
srcContents_ = &*srcContentsLock_;
}
}
deletedInode.reset();
}
auto sourceName = myPath + name;
auto targetName = destDirPath + newName;
getMount()->getJournal().wlock()->addDelta(
std::make_unique<JournalDelta>(JournalDelta{sourceName, targetName}));
return folly::Unit{};
void TreeInode::TreeRenameLocks::lockDestChild(PathComponentPiece destName) {
// Look up the destination child entry
destChildIter_ = destContents_->entries.find(destName);
if (destChildExists() && destChildIsDirectory() && destChild() != nullptr) {
auto* childTree = boost::polymorphic_downcast<TreeInode*>(destChild());
destChildContentsLock_ = childTree->contents_.wlock();
destChildContents_ = &*destChildContentsLock_;
}
}
InodeMap* TreeInode::getInodeMap() const {

17
eden/fs/inodes/TreeInode.h
View File

@ -146,14 +146,6 @@ class TreeInode : public InodeBase {
TreeInodePtr newParent,
PathComponentPiece newName);
std::unique_ptr<InodeBase> renameHelper(
Dir* sourceContents,
PathComponentPiece sourceName,
TreeInodePtr destParent,
Dir* destContents,
PathComponentPiece destName,
const RenameLock& renameLock);
fuse_ino_t getInode() const;
const folly::Synchronized<Dir>& getContents() const {
@ -235,6 +227,8 @@ class TreeInode : public InodeBase {
folly::Future<folly::Unit> loadMaterializedChildren();
private:
struct TreeRenameLocks;
void registerInodeLoadComplete(
folly::Future<std::unique_ptr<InodeBase>>& future,
PathComponentPiece name,
@ -251,6 +245,13 @@ class TreeInode : public InodeBase {
folly::Future<std::unique_ptr<InodeBase>>
startLoadingInode(Entry* entry, PathComponentPiece name, fuse_ino_t number);
folly::Future<folly::Unit> doRename(
TreeRenameLocks&& locks,
PathComponentPiece srcName,
PathMap<std::unique_ptr<Entry>>::iterator srcIter,
TreeInodePtr destParent,
PathComponentPiece destName);
/** Translates a Tree object from our store into a Dir object
* used to track the directory in the inode */
static Dir buildDirFromTree(const Tree* tree);

119
eden/fs/inodes/test/InodeMapTest.cpp
View File

@ -234,3 +234,122 @@ TEST(InodeMap, recursiveLookupError) {
EXPECT_THROW_RE(
fileFuture.get(), std::domain_error, "error for testing purposes");
}
TEST(InodeMap, renameDuringRecursiveLookup) {
BaseTestMountBuilder builder;
auto backingStore = builder.getBackingStore();
auto file = backingStore->putBlob("this is a test file");
auto d = backingStore->putTree({{"file.txt", file}});
auto c = backingStore->putTree({{"d", d}});
auto b = backingStore->putTree({{"c", c}});
auto a = backingStore->putTree({{"b", b}});
auto root = backingStore->putTree({{"a", a}});
builder.setCommit(makeTestHash("ccc"), root->get().getHash());
// build() will hang unless the root tree is ready.
root->setReady();
auto testMount = builder.build();
auto rootInode = testMount->getEdenMount()->getRootInode();
// Call EdenMount::getInode() on the root
auto rootFuture = testMount->getEdenMount()->getInode(RelativePathPiece{""});
ASSERT_TRUE(rootFuture.isReady());
auto rootResult = rootFuture.get();
EXPECT_EQ(rootInode, rootResult);
// Call EdenMount::getInode() to do a recursive lookup
auto fileFuture = testMount->getEdenMount()->getInode(
RelativePathPiece{"a/b/c/d/file.txt"});
EXPECT_FALSE(fileFuture.isReady());
c->setReady();
EXPECT_FALSE(fileFuture.isReady());
a->setReady();
EXPECT_FALSE(fileFuture.isReady());
b->setReady();
EXPECT_FALSE(fileFuture.isReady());
auto bFuture = testMount->getEdenMount()->getInode(RelativePathPiece{"a/b"});
ASSERT_TRUE(bFuture.isReady());
auto bInode = bFuture.get().asTreePtr();
// Rename c to x after the recursive resolution should have
// already looked it up
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(fileFuture.isReady());
// Now mark the rest of the tree ready
// Note that we don't actually have to mark the file itself ready.
// The Inode lookup itself doesn't need the blob data yet.
d->setReady();
ASSERT_TRUE(fileFuture.isReady());
auto fileInode = fileFuture.get();
// We should have successfully looked up the inode, but it will report it
// self (correctly) at its new path now.
EXPECT_EQ(
RelativePathPiece{"a/b/x/d/file.txt"}, fileInode->getPath().value());
}
TEST(InodeMap, renameDuringRecursiveLookupAndLoad) {
BaseTestMountBuilder builder;
auto backingStore = builder.getBackingStore();
auto file = backingStore->putBlob("this is a test file");
auto d = backingStore->putTree({{"file.txt", file}});
auto c = backingStore->putTree({{"d", d}});
auto b = backingStore->putTree({{"c", c}});
auto a = backingStore->putTree({{"b", b}});
auto root = backingStore->putTree({{"a", a}});
builder.setCommit(makeTestHash("ccc"), root->get().getHash());
// build() will hang unless the root tree is ready.
root->setReady();
auto testMount = builder.build();
auto rootInode = testMount->getEdenMount()->getRootInode();
// Call EdenMount::getInode() on the root
auto rootFuture = testMount->getEdenMount()->getInode(RelativePathPiece{""});
ASSERT_TRUE(rootFuture.isReady());
auto rootResult = rootFuture.get();
EXPECT_EQ(rootInode, rootResult);
// Call EdenMount::getInode() to do a recursive lookup
auto fileFuture = testMount->getEdenMount()->getInode(
RelativePathPiece{"a/b/c/d/file.txt"});
EXPECT_FALSE(fileFuture.isReady());
a->setReady();
EXPECT_FALSE(fileFuture.isReady());
b->setReady();
EXPECT_FALSE(fileFuture.isReady());
auto bFuture = testMount->getEdenMount()->getInode(RelativePathPiece{"a/b"});
ASSERT_TRUE(bFuture.isReady());
auto bInode = bFuture.get().asTreePtr();
// Rename c to x while the recursive resolution is still trying
// to look it up.
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
// The rename will not complete until C becomes ready
EXPECT_FALSE(renameFuture.isReady());
EXPECT_FALSE(fileFuture.isReady());
c->setReady();
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(fileFuture.isReady());
// Now mark the rest of the tree ready
// Note that we don't actually have to mark the file itself ready.
// The Inode lookup itself doesn't need the blob data yet.
d->setReady();
ASSERT_TRUE(fileFuture.isReady());
auto fileInode = fileFuture.get();
// We should have successfully looked up the inode, but it will report it
// self (correctly) at its new path now.
EXPECT_EQ(
RelativePathPiece{"a/b/x/d/file.txt"}, fileInode->getPath().value());
}

745
eden/fs/inodes/test/RenameTest.cpp Normal file
View File

@ -0,0 +1,745 @@
/*
* Copyright (c) 2016-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*/
#include "eden/fs/inodes/InodeMap.h"
#include <folly/Bits.h>
#include <folly/Format.h>
#include <folly/String.h>
#include <gtest/gtest.h>
#include "eden/fs/inodes/EdenMount.h"
#include "eden/fs/inodes/FileInode.h"
#include "eden/fs/inodes/TreeInode.h"
#include "eden/fs/testharness/FakeBackingStore.h"
#include "eden/fs/testharness/TestMount.h"
#include "eden/fs/testharness/TestUtil.h"
#include "eden/utils/Bug.h"
#include "eden/utils/test/TestChecks.h"
using namespace facebook::eden;
using folly::StringPiece;
class RenameTest : public ::testing::Test {
protected:
void SetUp() override {
// Set up a directory structure that we will use for most
// of the tests below
TestMountBuilder builder;
builder.addFiles({
{"a/b/c/doc.txt", "This file is used for most of the file renames.\n"},
{"a/readme.txt", "I exist to be replaced.\n"},
{"a/b/readme.txt", "I exist to be replaced.\n"},
{"a/b/c/readme.txt", "I exist to be replaced.\n"},
{"a/b/c/d/readme.txt", "I exist to be replaced.\n"},
{"a/b/c/d/e/f/readme.txt", "I exist to be replaced.\n"},
{"a/x/y/z/readme.txt", "I exist to be replaced.\n"},
});
mount_ = builder.build();
// Also create some empty directories for the tests
mount_->mkdir("a/emptydir");
mount_->mkdir("a/b/emptydir");
mount_->mkdir("a/b/c/emptydir");
mount_->mkdir("a/b/c/d/emptydir");
mount_->mkdir("a/b/c/d/e/f/emptydir");
mount_->mkdir("a/x/y/z/emptydir");
mount_->mkdir("a/b/c/1");
mount_->mkdir("a/b/c/1/2");
mount_->mkdir("a/b/c/1/emptydir");
mount_->mkdir("a/b/c/1/2/emptydir");
}
void
renameFile(StringPiece srcPathStr, StringPiece destPathStr, bool destExists);
void
renameDir(StringPiece srcPathStr, StringPiece destPathStr, bool destExists);
void renameError(
StringPiece srcPathStr,
StringPiece destPathStr,
int expectedError);
std::unique_ptr<TestMount> mount_;
};
/*
* Basic tests for renaming files
*/
void RenameTest::renameFile(
StringPiece srcPathStr,
StringPiece destPathStr,
bool destExists) {
RelativePath srcPath{srcPathStr};
auto srcBase = srcPath.basename();
RelativePath destPath{destPathStr};
auto destBase = destPath.basename();
// Get the file pre-rename
auto origSrc = mount_->getFileInode(srcPath);
EXPECT_EQ(srcPath, origSrc->getPath().value());
FileInodePtr origDest;
if (destExists) {
origDest = mount_->getFileInode(destPath);
EXPECT_EQ(destPath, origDest->getPath().value());
EXPECT_NE(origSrc->getNodeId(), origDest->getNodeId());
} else {
EXPECT_THROW_ERRNO(mount_->getFileInode(destPath), ENOENT);
}
// Do the rename
auto srcDir = mount_->getTreeInode(srcPath.dirname());
auto destDir = mount_->getTreeInode(destPath.dirname());
auto renameFuture = srcDir->rename(srcBase, destDir, destBase);
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// Now get the file post-rename
// Make sure it is the same inode, but the path is updated
auto renamedInode = mount_->getFileInode(destPath);
EXPECT_EQ(destPath, renamedInode->getPath().value());
EXPECT_EQ(origSrc->getNodeId(), renamedInode->getNodeId());
EXPECT_EQ(origSrc.get(), renamedInode.get());
EXPECT_EQ(destPath, origSrc->getPath().value());
// The original test file should now be unlinked
if (destExists) {
EXPECT_TRUE(origDest->isUnlinked());
}
// Trying to access the original name now should fail
EXPECT_THROW_ERRNO(mount_->getFileInode(srcPath), ENOENT);
}
TEST_F(RenameTest, renameFileSameDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/newdocs.txt", false);
}
TEST_F(RenameTest, renameFileParentDirectory) {
renameFile("a/b/c/doc.txt", "a/b/newdocs.txt", false);
}
TEST_F(RenameTest, renameFileChildDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/d/newdocs.txt", false);
}
TEST_F(RenameTest, renameFileAncestorDirectory) {
renameFile("a/b/c/doc.txt", "a/newdocs.txt", false);
}
TEST_F(RenameTest, renameFileDescendantDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/d/e/f/newdocs.txt", false);
}
TEST_F(RenameTest, renameFileOtherDirectory) {
renameFile("a/b/c/doc.txt", "a/x/y/z/newdocs.txt", false);
}
TEST_F(RenameTest, replaceFileSameDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/readme.txt", true);
}
TEST_F(RenameTest, replaceFileParentDirectory) {
renameFile("a/b/c/doc.txt", "a/b/readme.txt", true);
}
TEST_F(RenameTest, replaceFileChildDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/d/readme.txt", true);
}
TEST_F(RenameTest, replaceFileAncestorDirectory) {
renameFile("a/b/c/doc.txt", "a/readme.txt", true);
}
TEST_F(RenameTest, replaceFileDescendantDirectory) {
renameFile("a/b/c/doc.txt", "a/b/c/d/e/f/readme.txt", true);
}
TEST_F(RenameTest, replaceFileOtherDirectory) {
renameFile("a/b/c/doc.txt", "a/x/y/z/readme.txt", true);
}
TEST_F(RenameTest, renameFileToSamePath) {
RelativePath path{"a/b/c/doc.txt"};
// Get the file pre-rename
auto origFile = mount_->getFileInode(path);
EXPECT_EQ(path, origFile->getPath().value());
// Do the rename
auto parentDir = mount_->getTreeInode(path.dirname());
auto renameFuture =
parentDir->rename(path.basename(), parentDir, path.basename());
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// Just to be thorough, make sure looking up the path still returns the
// original inode.
auto renamedInode = mount_->getFileInode(path);
EXPECT_EQ(path, renamedInode->getPath().value());
EXPECT_EQ(origFile->getNodeId(), renamedInode->getNodeId());
EXPECT_EQ(origFile.get(), renamedInode.get());
EXPECT_EQ(path, origFile->getPath().value());
}
/*
* Basic tests for renaming directories
*/
void RenameTest::renameDir(
StringPiece srcPathStr,
StringPiece destPathStr,
bool destExists) {
RelativePath srcPath{srcPathStr};
auto srcBase = srcPath.basename();
RelativePath destPath{destPathStr};
auto destBase = destPath.basename();
// Get the trees pre-rename
auto origSrc = mount_->getTreeInode(srcPath);
EXPECT_EQ(srcPath, origSrc->getPath().value());
TreeInodePtr origDest;
if (destExists) {
origDest = mount_->getTreeInode(destPath);
EXPECT_EQ(destPath, origDest->getPath().value());
EXPECT_NE(origSrc->getNodeId(), origDest->getNodeId());
} else {
EXPECT_THROW_ERRNO(mount_->getTreeInode(destPath), ENOENT);
}
// Do the rename
auto srcDir = mount_->getTreeInode(srcPath.dirname());
auto destDir = mount_->getTreeInode(destPath.dirname());
auto renameFuture = srcDir->rename(srcBase, destDir, destBase);
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// Now get the file post-rename
// Make sure it is the same inode, but the path is updated
auto renamedInode = mount_->getTreeInode(destPath);
EXPECT_EQ(destPath, renamedInode->getPath().value());
EXPECT_EQ(origSrc->getNodeId(), renamedInode->getNodeId());
EXPECT_EQ(origSrc.get(), renamedInode.get());
EXPECT_EQ(destPath, origSrc->getPath().value());
// The original test file should now be unlinked
if (destExists) {
EXPECT_TRUE(origDest->isUnlinked());
}
// Trying to access the original name now should fail
EXPECT_THROW_ERRNO(mount_->getTreeInode(srcPath), ENOENT);
}
TEST_F(RenameTest, renameDirSameDirectory) {
renameDir("a/b/c/d", "a/b/c/newdir", false);
}
TEST_F(RenameTest, renameDirParentDirectory) {
renameDir("a/b/c/d", "a/b/newdir", false);
}
TEST_F(RenameTest, renameDirChildDirectory) {
renameDir("a/b/c/d", "a/b/c/1/newdir", false);
}
TEST_F(RenameTest, renameDirAncestorDirectory) {
renameDir("a/b/c/d", "a/newdir", false);
}
TEST_F(RenameTest, renameDirDescendantDirectory) {
renameDir("a/b/c/d", "a/b/c/1/2/newdir", false);
}
TEST_F(RenameTest, renameDirOtherDirectory) {
renameDir("a/b/c/d", "a/x/y/z/newdir", false);
}
TEST_F(RenameTest, replaceDirSameDirectory) {
renameDir("a/b/c/d", "a/b/c/emptydir", true);
}
TEST_F(RenameTest, replaceDirParentDirectory) {
renameDir("a/b/c/d", "a/b/emptydir", true);
}
TEST_F(RenameTest, replaceDirChildDirectory) {
renameDir("a/b/c/d", "a/b/c/1/emptydir", true);
}
TEST_F(RenameTest, replaceDirAncestorDirectory) {
renameDir("a/b/c/d", "a/emptydir", true);
}
TEST_F(RenameTest, replaceDirDescendantDirectory) {
renameDir("a/b/c/d", "a/b/c/1/2/emptydir", true);
}
TEST_F(RenameTest, replaceDirOtherDirectory) {
renameDir("a/b/c/d", "a/x/y/z/emptydir", true);
}
TEST_F(RenameTest, renameDirToSamePath) {
RelativePath path{"a/b/c/d"};
// Get the file pre-rename
auto origDir = mount_->getTreeInode(path);
EXPECT_EQ(path, origDir->getPath().value());
// Do the rename
auto parentDir = mount_->getTreeInode(path.dirname());
auto renameFuture =
parentDir->rename(path.basename(), parentDir, path.basename());
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// Just to be thorough, make sure looking up the path still returns the
// original inode.
auto renamedInode = mount_->getTreeInode(path);
EXPECT_EQ(path, renamedInode->getPath().value());
EXPECT_EQ(origDir->getNodeId(), renamedInode->getNodeId());
EXPECT_EQ(origDir.get(), renamedInode.get());
EXPECT_EQ(path, origDir->getPath().value());
}
/*
* Tests for error conditions
*/
void RenameTest::renameError(
StringPiece srcPathStr,
StringPiece destPathStr,
int expectedError) {
RelativePath srcPath{srcPathStr};
auto srcBase = srcPath.basename();
RelativePath destPath{destPathStr};
auto destBase = destPath.basename();
// Do the rename
auto srcDir = mount_->getTreeInode(srcPath.dirname());
auto destDir = mount_->getTreeInode(destPath.dirname());
auto renameFuture = srcDir->rename(srcBase, destDir, destBase);
// The rename should fail with the expected error
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_ERRNO(renameFuture.get(), expectedError);
}
TEST_F(RenameTest, renameNonexistentFile) {
renameError("a/b/c/foo.txt", "a/b/c/bar.txt", ENOENT);
}
TEST_F(RenameTest, renameFileOverEmptyDir) {
renameError("a/b/c/doc.txt", "a/b/c/emptydir", EISDIR);
}
TEST_F(RenameTest, renameFileOverNonEmptyDir) {
// For now we require EISDIR, although ENOTEMPTY also seems like it might be
// potentially acceptable.
renameError("a/b/c/doc.txt", "a/b/c/d", EISDIR);
}
TEST_F(RenameTest, renameDirOverFile) {
renameError("a/b/c/d", "a/b/c/doc.txt", ENOTDIR);
}
TEST_F(RenameTest, renameDirOverNonEmptyDir) {
renameError("a/b/c/1", "a/b/c/d", ENOTEMPTY);
}
/*
* Several tests for invalid rename paths.
* The linux kernel should make sure that invalid rename requests like
* this don't make it to us via FUSE, but check to make sure our code
* conservatively handles these errors anyway.
*/
TEST_F(RenameTest, renameToInvalidChildPath) {
renameError("a/b/c/d", "a/b/c/d/newdir", EINVAL);
}
TEST_F(RenameTest, renameToInvalidDescendentPath) {
renameError("a/b/c/d", "a/b/c/d/e/newdir", EINVAL);
}
TEST_F(RenameTest, renameToInvalidParentPath) {
renameError("a/b/c/d", "a/b/c", ENOTEMPTY);
}
TEST_F(RenameTest, renameToInvalidAncestorPath) {
renameError("a/b/c/d", "a/b", ENOTEMPTY);
}
TEST_F(RenameTest, renameIntoUnlinkedDir) {
RelativePath srcPath{"a/b/c/doc.txt"};
RelativePath destDirPath{"a/b/c/emptydir"};
// Look up the source and destination directories
auto srcDir = mount_->getTreeInode(srcPath.dirname());
auto destDir = mount_->getTreeInode(destDirPath);
// Now unlink the destination directory
auto destDirParent = mount_->getTreeInode(destDirPath.dirname());
auto rmdirFuture = destDirParent->rmdir(destDirPath.basename());
ASSERT_TRUE(rmdirFuture.isReady());
EXPECT_FALSE(rmdirFuture.hasException());
rmdirFuture.get();
// Do the rename
auto renameFuture = srcDir->rename(
srcPath.basename(), destDir, PathComponentPiece{"test.txt"});
// The rename should fail with ENOENT since the destination directory no
// longer exists
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_ERRNO(renameFuture.get(), ENOENT);
}
/*
* Rename tests where the source and destination inode objects
* are not loaded yet when the rename starts.
*/
class RenameLoadingTest : public ::testing::Test {
protected:
void SetUp() override {
// Set up a directory structure that we will use for most
// of the tests below
BaseTestMountBuilder builder;
auto backingStore = builder.getBackingStore();
// This sets up the following files:
// - a/b/c/doc.txt
// - a/b/c/readme.txt
// - a/b/testdir/sample.txt
// - a/b/empty/
doc_ = backingStore->putBlob("documentation\n");
readme_ = backingStore->putBlob("more docs\n");
sample_ = backingStore->putBlob("Lorem ipsum dolor sit amet\n");
a_b_c_ =
backingStore->putTree({{"doc.txt", doc_}, {"readme.txt", readme_}});
a_b_testdir_ = backingStore->putTree({{"sample.txt", sample_}});
// Empty directories generally aren't tracked by source control,
// but create one for testing purposes anyway.
a_b_empty_ = backingStore->putTree({});
a_b_ = backingStore->putTree(
{{"c", a_b_c_}, {"testdir", a_b_testdir_}, {"empty", a_b_empty_}});
a_ = backingStore->putTree({{"b", a_b_}});
root_ = backingStore->putTree({{"a", a_}});
builder.setCommit(makeTestHash("ccc"), root_->get().getHash());
// build() will hang unless the root tree is ready.
root_->setReady();
mount_ = builder.build();
}
std::unique_ptr<TestMount> mount_;
StoredBlob* doc_;
StoredBlob* readme_;
StoredBlob* sample_;
StoredTree* a_b_c_;
StoredTree* a_b_;
StoredTree* a_b_testdir_;
StoredTree* a_b_empty_;
StoredTree* a_;
StoredTree* root_;
};
TEST_F(RenameLoadingTest, renameDirSameDirectory) {
a_->setReady();
a_b_->setReady();
// Perform a rename where the child inode ("a/b/c" in this case)
// is not ready yet, because the data is not available from the BackingStore.
//
// For now we have to test this with a directory, and not a regular file,
// since file inodes can always be loaded immediately (as long as their
// parent inode is ready). File inodes do not wait to load the blob data
// from the backing store before creating the FileInode object.
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
// The rename will not complete until a_b_c_ becomes ready
EXPECT_FALSE(renameFuture.isReady());
// Now make a_b_c_ ready
a_b_c_->setReady();
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
}
TEST_F(RenameLoadingTest, renameWithLoadPending) {
a_->setReady();
a_b_->setReady();
// Start a lookup on a/b/c before we start the rename
auto inodeFuture =
mount_->getEdenMount()->getInode(RelativePathPiece{"a/b/c"});
EXPECT_FALSE(inodeFuture.isReady());
// Perform a rename on a/b/c before that inode is ready.
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
// The rename will not complete until a_b_c_ becomes ready
EXPECT_FALSE(renameFuture.isReady());
// Now make a_b_c_ ready
a_b_c_->setReady();
// Both the load and the rename should have completed
ASSERT_TRUE(inodeFuture.isReady());
ASSERT_TRUE(renameFuture.isReady());
// The rename should be successful
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// From an API guarantee point of view, it would be fine for the load
// to succeed or to fail with ENOENT here, since it was happening
// concurrently with a rename() that moved the file away from the path we
// requested.
//
// In practice our code currently always succeeds the load attempt.
if (inodeFuture.hasException()) {
EXPECT_THROW_ERRNO(inodeFuture.get(), ENOENT);
} else {
auto cInode = inodeFuture.get();
EXPECT_EQ("a/b/x", cInode->getPath().value().stringPiece());
}
}
TEST_F(RenameLoadingTest, loadWithRenamePending) {
a_->setReady();
a_b_->setReady();
// Perform a rename on a/b/c before that inode is ready.
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
// The rename will not complete until a_b_c_ becomes ready
EXPECT_FALSE(renameFuture.isReady());
// Also start a lookup on a/b/c after starting the rename
auto inodeFuture =
mount_->getEdenMount()->getInode(RelativePathPiece{"a/b/c"});
EXPECT_FALSE(inodeFuture.isReady());
// Now make a_b_c_ ready
a_b_c_->setReady();
// Both the load and the rename should have completed
ASSERT_TRUE(inodeFuture.isReady());
ASSERT_TRUE(renameFuture.isReady());
// The rename should be successful
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
// From an API guarantee point of view, it would be fine for the load
// to succeed or to fail with ENOENT here, since it was happening
// concurrently with a rename() that moved the file away from the path we
// requested.
//
// In practice our code currently always succeeds the load attempt.
if (inodeFuture.hasException()) {
EXPECT_THROW_ERRNO(inodeFuture.get(), ENOENT);
} else {
auto cInode = inodeFuture.get();
EXPECT_EQ("a/b/x", cInode->getPath().value().stringPiece());
}
}
TEST_F(RenameLoadingTest, renameLoadFailure) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture =
bInode->rename(PathComponentPiece{"c"}, bInode, PathComponentPiece{"x"});
// The rename will not complete until a_b_c_ becomes ready
EXPECT_FALSE(renameFuture.isReady());
// Fail the load of a_b_c_
a_b_c_->triggerError(std::domain_error("fake error for testing"));
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_RE(
renameFuture.get(), std::domain_error, "fake error for testing");
}
// Test a rename that replaces a destination directory, where neither
// the source nor destination are ready yet.
TEST_F(RenameLoadingTest, renameLoadDest) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"empty"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Make a_b_c_ ready first
a_b_c_->setReady();
EXPECT_FALSE(renameFuture.isReady());
// Now make a_b_empty_ ready
a_b_empty_->setReady();
// Both the load and the rename should have completed
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
}
TEST_F(RenameLoadingTest, renameLoadDestOtherOrder) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"empty"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Make a_b_empty_ ready first
a_b_empty_->setReady();
EXPECT_FALSE(renameFuture.isReady());
// Now make a_b_c_ ready
a_b_c_->setReady();
// Both the load and the rename should have completed
ASSERT_TRUE(renameFuture.isReady());
EXPECT_FALSE(renameFuture.hasException());
renameFuture.get();
}
// Test a rename that replaces a destination directory, where neither
// the source nor destination are ready yet.
TEST_F(RenameLoadingTest, renameLoadDestNonempty) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"testdir"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Make a_b_c_ ready first
a_b_c_->setReady();
EXPECT_FALSE(renameFuture.isReady());
// Now make a_b_testdir_ ready
a_b_testdir_->setReady();
// The load should fail with ENOTEMPTY
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_ERRNO(renameFuture.get(), ENOTEMPTY);
}
// Test a rename that replaces a destination directory, where neither
// the source nor destination are ready yet.
TEST_F(RenameLoadingTest, renameLoadDestNonemptyOtherOrder) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"testdir"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Make a_b_testdir_ ready first.
a_b_testdir_->setReady();
// The rename could potentially fail now, but it is also be fine for it to
// wait for the source directory to be ready too before it performs
// validation. Therefore go ahead and make the source directory ready too
// without checking renameFuture.isReady()
a_b_c_->setReady();
// The load should fail with ENOTEMPTY
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_ERRNO(renameFuture.get(), ENOTEMPTY);
}
TEST_F(RenameLoadingTest, renameLoadDestFailure) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"empty"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Make a_b_c_ ready first
a_b_c_->setReady();
EXPECT_FALSE(renameFuture.isReady());
// Now fail the load on a_b_empty_
a_b_empty_->triggerError(std::domain_error("fake error for testing"));
// Verify the rename failure
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_RE(
renameFuture.get(), std::domain_error, "fake error for testing");
}
TEST_F(RenameLoadingTest, renameLoadDestFailureOtherOrder) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"empty"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Fail the load on a_b_empty_ first
a_b_empty_->triggerError(std::domain_error("fake error for testing"));
// The rename may fail immediately, but it's also fine for it to wait
// for the source load to finish too. Therefore go ahead and finish the load
// on a_b_c_ without checking renameFuture.isReady()
a_b_c_->setReady();
// Verify the rename failure
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_RE(
renameFuture.get(), std::domain_error, "fake error for testing");
}
TEST_F(RenameLoadingTest, renameLoadBothFailure) {
a_->setReady();
a_b_->setReady();
// Perform a rename on "a/b/c" before it is ready
auto bInode = mount_->getTreeInode("a/b");
auto renameFuture = bInode->rename(
PathComponentPiece{"c"}, bInode, PathComponentPiece{"empty"});
// The rename will not complete until both a_b_c_ and a_b_empty_ become ready
EXPECT_FALSE(renameFuture.isReady());
// Trigger errors on both inode loads
a_b_c_->triggerError(std::domain_error("fake error for testing: src"));
a_b_empty_->triggerError(std::domain_error("fake error for testing: dest"));
// Verify the rename failure.
// It doesn't matter which error we got, as long as one of
// them was propated up. (In practice our code currently propagates the
// first error it receives.)
ASSERT_TRUE(renameFuture.isReady());
EXPECT_THROW_RE(
renameFuture.get(), std::domain_error, "fake error for testing: .*");
}

6
eden/fs/testharness/TestMount.h
View File

@ -42,11 +42,11 @@ struct TestMountFile {
bool operator==(const TestMountFile& other) const;
/**
* @param path is a StringPiece (rather than a RelativePath) for convenience
* @param p is a StringPiece (rather than a RelativePath) for convenience
* for creating instances of TestMountFile for unit tests.
*/
TestMountFile(folly::StringPiece path, std::string contents)
: path(path), contents(std::move(contents)) {}
TestMountFile(folly::StringPiece p, folly::StringPiece c)
: path(p), contents(c.str()) {}
};
class TestMount {