search

cberner / fuser

Filesystem in Userspace (FUSE) for Rust
MIT License
836 stars 114 forks source link

Strange safety API regarding fuser::spawn_mount #195

Closed vi closed 2 years ago

vi commented 2 years ago

It is a safe function, yet with a "Safety" paragraph in documentation that is supposed to be only present in unsafe fns.

Also it states "should not be leaked" without constituting what exactly is a leak. Is using spawn_mount, then sending away resulting BackgroundSession to other thread (it is Send and Sync), then allowing original thread to exit, then dropping BackgroundSession in the new thread safe? What exactly could happen if cleanup is skipped?

The doccomment refers to old std::thread::JoinGuard issue, but I do not see it used in the code, just usual JoinHandle. If only correctness at stake (i.e. not leaving a "Endpoint is not connected" filesystem behind or some deadlocking) then "Safety" should be renamed to "Correctness".

If it is indeed about safely, why not use closure approach with a signature like this then:

pub fn mount_unmountable<...>(
    filesystem: FS, 
    mountpoint: P, 
    options: &[&OsStr],
    unmounter: impl FnOnce(&UnmountHandle),
) -> Result<()>
cberner commented 2 years ago

That comment is from before I started maintaining fuser. However, to the best of my knowledge it's outdated and spawn_mount() is safe. I'll remove the comment