diff --git a/eden/fs/inodes/EdenMount.cpp b/eden/fs/inodes/EdenMount.cpp index d264ab97ad..0bdd8c308c 100644 --- a/eden/fs/inodes/EdenMount.cpp +++ b/eden/fs/inodes/EdenMount.cpp @@ -494,9 +494,10 @@ folly::Future 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) diff --git a/eden/fs/inodes/EdenMount.h b/eden/fs/inodes/EdenMount.h index 75874c2b6b..4fbbe0d8fb 100644 --- a/eden/fs/inodes/EdenMount.h +++ b/eden/fs/inodes/EdenMount.h @@ -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 unmount(); diff --git a/eden/fs/inodes/test/EdenMountTest.cpp b/eden/fs/inodes/test/EdenMountTest.cpp index 070ff28e8a..9f980d4df4 100644 --- a/eden/fs/inodes/test/EdenMountTest.cpp +++ b/eden/fs/inodes/test/EdenMountTest.cpp @@ -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(); + 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(); - EXPECT_THROW( - { - mount.takeoverFuse(FuseChannelData{fuse->start(), {}}); - }, - EdenMountCancelled); -} - -TEST(EdenMount, takeoverFuseFailsIfUnmountIsPending) { - auto testMount = TestMount{FakeTreeBuilder{}}; - auto& mount = *testMount.getEdenMount(); - auto mountDelegate = std::make_shared(); - 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(); EXPECT_THROW( {