disallow infinite recursion in redirect unmount

Summary: If the bind unmount fails in in the privhelper, theres a possibility of infinite recursion in this method. This adds a flag to indicate if we've tried the bind unmount before.

Differential Revision: D30732857

fbshipit-source-id: 6ee887d211977ee94c8e66531287f076a7e61a2c

eden/fs/cli/redirect.py

@@ -377,7 +377,9 @@ class Redirection:
 
         raise Exception(f"don't know how to handle bind mounts on {sys.platform}")
 
-    def remove_existing(self, checkout: EdenCheckout) -> RepoPathDisposition:
+    def remove_existing(
+        self, checkout: EdenCheckout, fail_if_bind_mount: bool = False
+    ) -> RepoPathDisposition:
         repo_path = self.expand_repo_path(checkout)
         disposition = RepoPathDisposition.analyze(repo_path)
         if disposition == RepoPathDisposition.DOES_NOT_EXIST:
@@ -394,10 +396,16 @@ class Redirection:
             repo_path.unlink()
             return RepoPathDisposition.DOES_NOT_EXIST
         if disposition == RepoPathDisposition.IS_BIND_MOUNT:
+            if fail_if_bind_mount:
+                raise Exception(
+                    f"Failed to remove {repo_path} since the bind unmount failed"
+                )
             self._bind_unmount(checkout)
             # Now that it is unmounted, re-assess and ideally
             # remove the empty directory that was the mount point
-            return self.remove_existing(checkout)
+            # To avoid infinite recursion, tell the next call to fail if
+            # the disposition is still a bind mount
+            return self.remove_existing(checkout, True)
         if disposition == RepoPathDisposition.IS_EMPTY_DIR:
             repo_path.rmdir()
             return RepoPathDisposition.DOES_NOT_EXIST

eden/fs/cli/test/redirect_test.py

@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+# Copyright (c) Facebook, Inc. and its affiliates.
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2.
+
+# pyre-strict
+
+import os
+import unittest
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+from eden.fs.cli.doctor.test.lib.fake_eden_instance import FakeEdenInstance
+from eden.fs.cli.redirect import RedirectionType, RedirectionState
+from eden.test_support.temporary_directory import TemporaryDirectoryMixin
+
+from ..redirect import RepoPathDisposition, Redirection
+
+
+class RedirectTest(unittest.TestCase, TemporaryDirectoryMixin):
+    @patch("eden.fs.cli.redirect.Redirection._bind_unmount")
+    @patch("eden.fs.cli.redirect.RepoPathDisposition.analyze")
+    @patch("eden.fs.cli.redirect.Redirection.expand_repo_path")
+    @patch("eden.fs.cli.buck.is_buckd_running_for_path")
+    def test_twice_failed_bind_unmount(
+        self,
+        mock_buckd_running: MagicMock,
+        mock_expand_path: MagicMock,
+        mock_analyze: MagicMock,
+        mock_bind_unmount: MagicMock,
+    ) -> None:
+        temp_dir = self.make_temporary_directory()
+        repo_path = os.path.join(temp_dir, "test")
+
+        mock_bind_unmount.return_value = None
+        mock_analyze.return_value = RepoPathDisposition.IS_BIND_MOUNT
+        mock_expand_path.return_value = Path(repo_path)
+        mock_buckd_running.return_value = False
+
+        instance = FakeEdenInstance(temp_dir)
+        checkout = instance.create_test_mount("mount_dir")
+        redir = Redirection(
+            repo_path=Path(repo_path),
+            redir_type=RedirectionType.BIND,
+            target=None,
+            source="mount",
+            state=RedirectionState.UNKNOWN_MOUNT,
+        )
+
+        with self.assertRaises(Exception) as ex:
+            redir.remove_existing(checkout)
+
+        error_msg = f"Failed to remove {repo_path} since the bind unmount failed"
+        self.assertEqual(str(ex.exception), error_msg)