facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-07 15:27:13 +03:00
Code Issues Projects Releases Wiki Activity

Synchronize mount+unmount 3/4: Make unmount succeed if not mounted

Browse Source 
Summary:
I want to make EdenMount robust if mounting and unmounting are requested concurrently. This will make it safer to call EdenMount::destroy in different situations.

Take a step toward that goal by making EdenMount::unmount succeed if EdenMount::startFuse was never called.

Changed behaviors:

* If neither EdenMount::startFuse nor EdenMount::takeoverFuse was never called, EdenMount::unmount succeeds and does not call PrivHelper::fuseUnmount.
  * **Old behavior**: EdenMount::unmount fails with an exception from PrivHelper.

Unchanged behaviors:

* If EdenMount::unmount was ever called, EdenMount::startFuse fails with EdenMountCancelled.
* If EdenMount::unmount was ever called, EdenMount::takeoverFuse fails with EdenMountCancelled.
* If EdenMount::startFuse is in progress, EdenMount::unmount causes the concurrent startFuse call to fail with FuseDeviceUnmountedDuringInitialization.
* If EdenMount::startFuse -> PrivHelper::fuseMount is in progress, EdenMount::unmount waits for fuseMount to finish before calling PrivHelper::fuseUnmount.
* If EdenMount::startFuse -> PrivHelper::fuseMount is complete, EdenMount::unmount calls PrivHelper::fuseUnmount immediately.
* If EdenMount::takeoverFuse is in progress, EdenMount::unmount does not cause EdenMount::takeoverFuse to fail.
* if EdenMount::takeoverFuse was called, EdenMount::unmount calls PrivHelper::fuseUnmount immediately.
* If EdenMount::unmount is called multiple times, one of the calls succeeds, and the remaining calls to unmount fail with an exception from PrivHelper.

Reviewed By: simpkins

Differential Revision: D14398537

fbshipit-source-id: 9209aad8b2980045fd0f89a5ba149a833579c2cf
This commit is contained in:
Matt Glazar 2019-03-21 13:46:54 -07:00 committed by Facebook Github Bot
parent f0a6413df7
commit 7697a170be
3 changed files with 22 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
eden/fs/inodes/EdenMount.cpp
View File

@ -494,9 +494,10 @@ folly::Future<folly::Unit> EdenMount::unmount() {
return folly::makeFutureWith([this] {
auto mountingUnmountingState = mountingUnmountingState_.wlock();
mountingUnmountingState->unmountStarted = true;
auto mountFuture = mountingUnmountingState->fuseMountStarted()
? mountingUnmountingState->fuseMountPromise->getFuture()
: folly::makeFuture();
if (!mountingUnmountingState->fuseMountStarted()) {
return folly::makeFuture();
}
auto mountFuture = mountingUnmountingState->fuseMountPromise->getFuture();
mountingUnmountingState.unlock();
return std::move(mountFuture)

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

@ -176,6 +176,10 @@ class EdenMount {
*
* If startFuse() is in progress, unmount() might wait for startFuse() to
* finish before calling umount(2).
*
* If neither startFuse() nor takeoverFuse() has been called, unmount()
* finishes successfully without calling umount(2). Thereafter, startFuse()
* and takeoverFuse() will both fail with an EdenMountCancelled exception.
*/
FOLLY_NODISCARD folly::Future<folly::Unit> unmount();

46
eden/fs/inodes/test/EdenMountTest.cpp
View File

@ -644,6 +644,18 @@ TEST(
EXPECT_TRUE(mountDestroyDetector.mountIsDeleted());
}
TEST(EdenMount, unmountSucceedsIfNeverMounted) {
auto testMount = TestMount{FakeTreeBuilder{}};
auto& mount = *testMount.getEdenMount();
auto mountDelegate = std::make_shared<MockMountDelegate>();
testMount.getPrivHelper()->registerMountDelegate(
mount.getPath(), mountDelegate);
mount.unmount().get(kTimeout);
EXPECT_FALSE(mountDelegate->wasFuseUnmountEverCalled())
<< "unmount should not call fuseUnmount";
}
TEST(EdenMount, unmountDoesNothingIfPriorMountFailed) {
auto testMount = TestMount{FakeTreeBuilder{}};
auto& mount = *testMount.getEdenMount();
@ -844,10 +856,7 @@ TEST(EdenMount, startingFuseFailsImmediatelyIfUnmountWasEverCalled) {
testMount.getPrivHelper()->registerMountDelegate(
mount.getPath(), mountDelegate);
try {
mount.unmount().within(kTimeout).get();
} catch (MockMountDelegate::UnmountFailed&) {
}
mount.unmount().within(kTimeout).get();
EXPECT_THROW(mount.startFuse().get(kTimeout), EdenMountCancelled);
EXPECT_FALSE(mountDelegate->wasFuseMountEverCalled())
@ -861,34 +870,7 @@ TEST(EdenMount, takeoverFuseFailsIfUnmountWasEverCalled) {
testMount.getPrivHelper()->registerMountDelegate(
mount.getPath(), mountDelegate);
try {
mount.unmount().within(kTimeout).get();
} catch (MockMountDelegate::UnmountFailed&) {
}
auto fuse = std::make_shared<FakeFuse>();
EXPECT_THROW(
{
mount.takeoverFuse(FuseChannelData{fuse->start(), {}});
},
EdenMountCancelled);
}
TEST(EdenMount, takeoverFuseFailsIfUnmountIsPending) {
auto testMount = TestMount{FakeTreeBuilder{}};
auto& mount = *testMount.getEdenMount();
auto mountDelegate = std::make_shared<MockMountDelegate>();
testMount.getPrivHelper()->registerMountDelegate(
mount.getPath(), mountDelegate);
auto unmountPromise = mountDelegate->makeUnmountPromise();
auto unmountFuture = mount.unmount();
ASSERT_FALSE(unmountFuture.wait(kMicroTimeout).isReady())
<< "unmount should not finish until fuseUnmount completes";
SCOPE_EXIT {
unmountPromise.setValue();
std::move(unmountFuture).get(kTimeout);
};
mount.unmount().within(kTimeout).get();
auto fuse = std::make_shared<FakeFuse>();
EXPECT_THROW(
{