skip umount if session has been destroyed

src/mnt/fuse2.rs

@@ -6,7 +6,10 @@ use std::{
     io,
     os::unix::prelude::{FromRawFd, OsStrExt},
     path::Path,
-    sync::Arc,
+    sync::{
+        atomic::{AtomicBool, Ordering},
+        Arc,
+    },
 };
 
 /// Ensures that an os error is never 0/Success
@@ -21,17 +24,28 @@ fn ensure_last_os_error() -> io::Error {
 #[derive(Debug)]
 pub struct Mount {
     mountpoint: CString,
+    destroyed: Arc<AtomicBool>,
 }
 impl Mount {
-    pub fn new(mountpoint: &Path, options: &[MountOption]) -> io::Result<(Arc<File>, Mount)> {
+    pub fn new(
+        mountpoint: &Path,
+        options: &[MountOption],
+        destroyed: Arc<AtomicBool>,
+    ) -> io::Result<(Arc<File>, Mount)> {
         let mountpoint = CString::new(mountpoint.as_os_str().as_bytes()).unwrap();
         with_fuse_args(options, |args| {
             let fd = unsafe { fuse_mount_compat25(mountpoint.as_ptr(), args) };
             if fd < 0 {
                 Err(ensure_last_os_error())
             } else {
                 let file = unsafe { File::from_raw_fd(fd) };
-                Ok((Arc::new(file), Mount { mountpoint }))
+                Ok((
+                    Arc::new(file),
+                    Mount {
+                        mountpoint,
+                        destroyed,
+                    },
+                ))
             }
         })
     }
@@ -46,23 +60,25 @@ impl Drop for Mount {
         // no indication of the error available to the caller. So we call unmount
         // directly, which is what osxfuse does anyway, since we already converted
         // to the real path when we first mounted.
-        if let Err(err) = super::libc_umount(&self.mountpoint) {
-            // Linux always returns EPERM for non-root users.  We have to let the
-            // library go through the setuid-root "fusermount -u" to unmount.
-            if err.kind() == PermissionDenied {
-                #[cfg(not(any(
-                    target_os = "macos",
-                    target_os = "freebsd",
-                    target_os = "dragonfly",
-                    target_os = "openbsd",
-                    target_os = "netbsd"
-                )))]
-                unsafe {
-                    fuse_unmount_compat22(self.mountpoint.as_ptr());
-                    return;
+        if !self.destroyed.load(Ordering::Relaxed) {
+            if let Err(err) = super::libc_umount(&self.mountpoint) {
+                // Linux always returns EPERM for non-root users.  We have to let the
+                // library go through the setuid-root "fusermount -u" to unmount.
+                if err.kind() == PermissionDenied {
+                    #[cfg(not(any(
+                        target_os = "macos",
+                        target_os = "freebsd",
+                        target_os = "dragonfly",
+                        target_os = "openbsd",
+                        target_os = "netbsd"
+                    )))]
+                    unsafe {
+                        fuse_unmount_compat22(self.mountpoint.as_ptr());
+                        return;
+                    }
                 }
+                warn!("umount failed with {:?}", err);
             }
-            warn!("umount failed with {:?}", err);
         }
     }
 }

src/mnt/fuse3.rs

@@ -10,7 +10,10 @@ use std::{
     os::unix::{ffi::OsStrExt, io::FromRawFd},
     path::Path,
     ptr,
-    sync::Arc,
+    sync::{
+        atomic::{AtomicBool, Ordering},
+        Arc,
+    },
 };
 
 /// Ensures that an os error is never 0/Success
@@ -25,16 +28,24 @@ fn ensure_last_os_error() -> io::Error {
 #[derive(Debug)]
 pub struct Mount {
     fuse_session: *mut c_void,
+    destroyed: Arc<AtomicBool>,
 }
 impl Mount {
-    pub fn new(mnt: &Path, options: &[MountOption]) -> io::Result<(Arc<File>, Mount)> {
+    pub fn new(
+        mnt: &Path,
+        options: &[MountOption],
+        destroyed: Arc<AtomicBool>,
+    ) -> io::Result<(Arc<File>, Mount)> {
         let mnt = CString::new(mnt.as_os_str().as_bytes()).unwrap();
         with_fuse_args(options, |args| {
             let fuse_session = unsafe { fuse_session_new(args, ptr::null(), 0, ptr::null_mut()) };
             if fuse_session.is_null() {
                 return Err(io::Error::last_os_error());
             }
-            let mount = Mount { fuse_session };
+            let mount = Mount {
+                fuse_session,
+                destroyed,
+            };
             let result = unsafe { fuse_session_mount(mount.fuse_session, mnt.as_ptr()) };
             if result != 0 {
                 return Err(ensure_last_os_error());
@@ -53,9 +64,11 @@ impl Mount {
 }
 impl Drop for Mount {
     fn drop(&mut self) {
-        unsafe {
-            fuse_session_unmount(self.fuse_session);
-            fuse_session_destroy(self.fuse_session);
+        if !self.destroyed.load(Ordering::Relaxed) {
+            unsafe {
+                fuse_session_unmount(self.fuse_session);
+                fuse_session_destroy(self.fuse_session);
+            }
         }
     }
 }

src/mnt/fuse_pure.rs

@@ -20,6 +20,7 @@ use std::os::unix::io::{AsRawFd, FromRawFd};
 use std::os::unix::net::UnixStream;
 use std::path::Path;
 use std::process::{Command, Stdio};
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::{mem, ptr};
 
@@ -32,9 +33,14 @@ pub struct Mount {
     mountpoint: CString,
     auto_unmount_socket: Option<UnixStream>,
     fuse_device: Arc<File>,
+    destroyed: Arc<AtomicBool>,
 }
 impl Mount {
-    pub fn new(mountpoint: &Path, options: &[MountOption]) -> io::Result<(Arc<File>, Mount)> {
+    pub fn new(
+        mountpoint: &Path,
+        options: &[MountOption],
+        destroyed: Arc<AtomicBool>,
+    ) -> io::Result<(Arc<File>, Mount)> {
         let mountpoint = mountpoint.canonicalize()?;
         let (file, sock) = fuse_mount_pure(mountpoint.as_os_str(), options)?;
         let file = Arc::new(file);
@@ -44,6 +50,7 @@ impl Mount {
                 mountpoint: CString::new(mountpoint.as_os_str().as_bytes())?,
                 auto_unmount_socket: sock,
                 fuse_device: file,
+                destroyed,
             },
         ))
     }
@@ -52,24 +59,26 @@ impl Mount {
 impl Drop for Mount {
     fn drop(&mut self) {
         use std::io::ErrorKind::PermissionDenied;
-        if !is_mounted(&self.fuse_device) {
-            // If the filesystem has already been unmounted, avoid unmounting it again.
-            // Unmounting it a second time could cause a race with a newly mounted filesystem
-            // living at the same mountpoint
-            return;
-        }
-        if let Some(sock) = mem::take(&mut self.auto_unmount_socket) {
-            drop(sock);
-            // fusermount in auto-unmount mode, no more work to do.
-            return;
-        }
-        if let Err(err) = super::libc_umount(&self.mountpoint) {
-            if err.kind() == PermissionDenied {
-                // Linux always returns EPERM for non-root users.  We have to let the
-                // library go through the setuid-root "fusermount -u" to unmount.
-                fuse_unmount_pure(&self.mountpoint)
-            } else {
-                error!("Unmount failed: {}", err)
+        if !self.destroyed.load(Ordering::Relaxed) {
+            if !is_mounted(&self.fuse_device) {
+                // If the filesystem has already been unmounted, avoid unmounting it again.
+                // Unmounting it a second time could cause a race with a newly mounted filesystem
+                // living at the same mountpoint
+                return;
+            }
+            if let Some(sock) = mem::take(&mut self.auto_unmount_socket) {
+                drop(sock);
+                // fusermount in auto-unmount mode, no more work to do.
+                return;
+            }
+            if let Err(err) = super::libc_umount(&self.mountpoint) {
+                if err.kind() == PermissionDenied {
+                    // Linux always returns EPERM for non-root users.  We have to let the
+                    // library go through the setuid-root "fusermount -u" to unmount.
+                    fuse_unmount_pure(&self.mountpoint)
+                } else {
+                    error!("Unmount failed: {}", err)
+                }
             }
         }
     }

src/mnt/mod.rs

@@ -160,7 +160,7 @@ mod test {
         // want to try and clean up the directory if it's a mountpoint otherwise we'll
         // deadlock.
         let tmp = ManuallyDrop::new(tempfile::tempdir().unwrap());
-        let (file, mount) = Mount::new(tmp.path(), &[]).unwrap();
+        let (file, mount) = Mount::new(tmp.path(), &[], Default::default()).unwrap();
         let mnt = cmd_mount();
         eprintln!("Our mountpoint: {:?}\nfuse mounts:\n{}", tmp.path(), mnt,);
         assert!(mnt.contains(&*tmp.path().to_string_lossy()));

src/request.rs

@@ -11,6 +11,7 @@ use std::convert::TryFrom;
 #[cfg(feature = "abi-7-28")]
 use std::convert::TryInto;
 use std::path::Path;
+use std::sync::atomic::Ordering;
 
 use crate::channel::ChannelSender;
 use crate::ll::Request as _;
@@ -181,11 +182,11 @@ impl<'a> Request<'a> {
             // Filesystem destroyed
             ll::Operation::Destroy(x) => {
                 se.filesystem.destroy();
-                se.destroyed = true;
+                se.destroyed.store(true, Ordering::Relaxed);
                 return Ok(Some(x.reply()));
             }
             // Any operation is invalid after destroy
-            _ if se.destroyed => {
+            _ if se.destroyed() => {
                 warn!("Ignoring FUSE operation after destroy: {}", self.request);
                 return Err(Errno::EIO);
             }

src/session.rs

@@ -11,6 +11,7 @@ use nix::unistd::geteuid;
 use std::fmt;
 use std::os::fd::{AsFd, BorrowedFd, OwnedFd};
 use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 use std::thread::{self, JoinHandle};
 use std::{io, ops::DerefMut};
@@ -65,7 +66,7 @@ pub struct Session<FS: Filesystem> {
     /// True if the filesystem is initialized (init operation done)
     pub(crate) initialized: bool,
     /// True if the filesystem was destroyed (destroy operation done)
-    pub(crate) destroyed: bool,
+    pub(crate) destroyed: Arc<AtomicBool>,
 }
 
 impl<FS: Filesystem> AsFd for Session<FS> {
@@ -83,6 +84,7 @@ impl<FS: Filesystem> Session<FS> {
     ) -> io::Result<Session<FS>> {
         let mountpoint = mountpoint.as_ref();
         info!("Mounting {}", mountpoint.display());
+        let destroyed = Arc::new(AtomicBool::new(false));
         // If AutoUnmount is requested, but not AllowRoot or AllowOther we enforce the ACL
         // ourself and implicitly set AllowOther because fusermount needs allow_root or allow_other
         // to handle the auto_unmount option
@@ -93,9 +95,9 @@ impl<FS: Filesystem> Session<FS> {
             warn!("Given auto_unmount without allow_root or allow_other; adding allow_other, with userspace permission handling");
             let mut modified_options = options.to_vec();
             modified_options.push(MountOption::AllowOther);
-            Mount::new(mountpoint, &modified_options)?
+            Mount::new(mountpoint, &modified_options, destroyed.clone())?
         } else {
-            Mount::new(mountpoint, options)?
+            Mount::new(mountpoint, options, destroyed.clone())?
         };
 
         let ch = Channel::new(file);
@@ -116,7 +118,7 @@ impl<FS: Filesystem> Session<FS> {
             proto_major: 0,
             proto_minor: 0,
             initialized: false,
-            destroyed: false,
+            destroyed,
         })
     }
 
@@ -133,7 +135,7 @@ impl<FS: Filesystem> Session<FS> {
             proto_major: 0,
             proto_minor: 0,
             initialized: false,
-            destroyed: false,
+            destroyed: Arc::new(AtomicBool::new(false)),
         }
     }
 
@@ -193,6 +195,10 @@ impl<FS: Filesystem> Session<FS> {
     pub fn notifier(&self) -> Notifier {
         Notifier::new(self.ch.sender())
     }
+
+    pub(crate) fn destroyed(&self) -> bool {
+        self.destroyed.load(Ordering::Relaxed)
+    }
 }
 
 #[derive(Debug)]
@@ -227,9 +233,9 @@ impl<FS: 'static + Filesystem + Send> Session<FS> {
 
 impl<FS: Filesystem> Drop for Session<FS> {
     fn drop(&mut self) {
-        if !self.destroyed {
+        if !self.destroyed() {
             self.filesystem.destroy();
-            self.destroyed = true;
+            self.destroyed.store(true, Ordering::Relaxed);
         }
 
         if let Some((mountpoint, _mount)) = std::mem::take(&mut *self.mount.lock().unwrap()) {