From b666ebcdd616691021bd625d76505ce1fe8027e4 Mon Sep 17 00:00:00 2001 From: Chad Austin Date: Thu, 18 Apr 2019 14:54:48 -0700 Subject: [PATCH] bypass cache when checking stale mounts Summary: Some Eden users observed that eden doctor wasn't cleaning up the stale mounts left from a crashed (or oomkilled) edenfs process. I reproduced by running eden doctor, killing the privhelper, then the main edenfs process, then running eden doctor again. Reviewed By: simpkins Differential Revision: D14991043 fbshipit-source-id: 72e771bd556424b5c9da6be178afbf337fbfe4ad --- eden/cli/doctor/check_stale_mounts.py | 15 ++++++------- eden/cli/doctor/test/lib/fake_mount_table.py | 11 ++++++++++ eden/cli/mtab.py | 22 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/eden/cli/doctor/check_stale_mounts.py b/eden/cli/doctor/check_stale_mounts.py index 683779a93a..5b4cd12372 100644 --- a/eden/cli/doctor/check_stale_mounts.py +++ b/eden/cli/doctor/check_stale_mounts.py @@ -79,14 +79,15 @@ def get_all_stale_eden_mount_points(mount_table: mtab.MountTable) -> List[bytes] log = logging.getLogger("eden.cli.doctor.stale_mounts") stale_eden_mount_points: Set[bytes] = set() for mount_point in get_all_eden_mount_points(mount_table): + # All eden mounts should have a .eden directory. + # If the edenfs daemon serving this mount point has died we + # will get ENOTCONN when trying to access it. (Simply calling + # lstat() on the root directory itself can succeed even in this + # case.) + eden_dir = os.path.join(mount_point, b".eden") + try: - # All eden mounts should have a .eden directory. - # If the edenfs daemon serving this mount point has died we - # will get ENOTCONN when trying to access it. (Simply calling - # lstat() on the root directory itself can succeed even in this - # case.) - eden_dir = os.path.join(mount_point, b".eden") - mount_table.lstat(eden_dir) + mount_table.check_path_access(eden_dir) except OSError as e: if e.errno == errno.ENOTCONN: stale_eden_mount_points.add(mount_point) diff --git a/eden/cli/doctor/test/lib/fake_mount_table.py b/eden/cli/doctor/test/lib/fake_mount_table.py index e6f76c36f5..c3a9e48901 100644 --- a/eden/cli/doctor/test/lib/fake_mount_table.py +++ b/eden/cli/doctor/test/lib/fake_mount_table.py @@ -113,6 +113,17 @@ class FakeMountTable(mtab.MountTable): else: return typing.cast(mtab.MTStat, result) + def check_path_access(self, path: bytes) -> None: + path_str = os.fsdecode(path) + + try: + result = self.stats[path_str] + except KeyError: + raise OSError(errno.ENOENT, f"no path {path_str}") + + if isinstance(result, BaseException): + raise result + def _remove_mount(self, mount_point: bytes) -> None: self.mounts[:] = [ mount_info diff --git a/eden/cli/mtab.py b/eden/cli/mtab.py index 7cce1cee32..a5774f208a 100644 --- a/eden/cli/mtab.py +++ b/eden/cli/mtab.py @@ -7,8 +7,10 @@ # of patent rights can be found in the PATENTS file in the same directory. import abc +import errno import logging import os +import random import subprocess from typing import List, NamedTuple, Union @@ -41,6 +43,13 @@ class MountTable(abc.ABC): def lstat(self, path: Union[bytes, str]) -> MTStat: "Returns a subset of the results of os.lstat." + @abc.abstractmethod + def check_path_access(self, path: bytes) -> None: + """\ + Attempts to stat the given directory, bypassing the kernel's caches. + Raises OSError upon failure. + """ + @abc.abstractmethod def create_bind_mount(self, source_path, dest_path) -> bool: "Creates a bind mount from source_path to dest_path." @@ -82,6 +91,19 @@ class LinuxMountTable(MountTable): st = os.lstat(path) return MTStat(st_uid=st.st_uid, st_dev=st.st_dev, st_mode=st.st_mode) + def check_path_access(self, path: bytes) -> None: + # Even if the FUSE process is shut down, the lstat call will succeed if + # the stat result is cached. Append a random string to avoid that. In a + # better world, this code would bypass the cache by opening a handle + # with O_DIRECT, but Eden does not support O_DIRECT. + + try: + os.lstat(os.path.join(path, hex(random.getrandbits(32))[2:].encode())) + except OSError as e: + if e.errno == errno.ENOENT: + return + raise + def create_bind_mount(self, source_path, dest_path) -> bool: return 0 == subprocess.check_call( ["sudo", "mount", "-o", "bind", source_path, dest_path]