Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

destroy() not being called on Linux #153

Closed
mgree opened this issue Jun 16, 2021 · 10 comments · Fixed by #154
Closed

destroy() not being called on Linux #153

mgree opened this issue Jun 16, 2021 · 10 comments · Fixed by #154

Comments

@mgree
Copy link
Contributor

mgree commented Jun 16, 2021

I'm using fuser on macOS and Linux. My filesystem writes data back out when destroy() is called, which works fine on macOS. On Linux, it seems like destroy() is never getting called.

This issue may be old: zargony/fuse-rs#151. Others have had issues with libfuse not properly calling destroy due to signal handlers.

You can see a successful run on macOS; the relevant "calling sync" lines don't appear on Linux.

I had suspected that calling fusermount -u instead of umount was the key, but it makes no difference. Killing the process doesn't help either.

@mgree
Copy link
Contributor Author

mgree commented Jun 17, 2021

A workaround (in mgree/ffs#8): I use Drop::drop instead of Filesystem::destroy to write my output.

@cberner
Copy link
Owner

cberner commented Jun 18, 2021

Thanks for the report. Ya, I can confirm that I don't see destroy called. I'm looking into whether it works with a naive C filesystem built with libfuse

@mgree
Copy link
Contributor Author

mgree commented Jun 19, 2021

Awesome---thanks for the super fast turnaround on this! 🎉

Glancing at the change log in #154, am I right to understand that the contract with destroy() is that it will be called exactly once?

@cberner
Copy link
Owner

cberner commented Jun 19, 2021

Yes, it should now be called exactly once, and no other methods should be called after destroy()

@mgree
Copy link
Contributor Author

mgree commented Sep 24, 2021

I'm working on upgrading ffs to fuser 0.9.1 (fuser-0.9.1 branch), but I'm getting some weird behaviors on Linux---notably that destroy isn't getting called, and I'm now getting an OS error when drop is called.

You can see an example in my CI logs, where there's no log from destroy and on os error 14 (EFAULT, bad address).

Any idea what could be going wrong here?

@cberner
Copy link
Owner

cberner commented Sep 25, 2021

Hmm, can you check if this line prints in your log, with logging enabled? https://github.com/cberner/fuser/blob/master/src/session.rs#L180

Also, are you mounting the filesystem with mount or spawn_mount?

@mgree
Copy link
Contributor Author

mgree commented Sep 25, 2021

@mgree
Copy link
Contributor Author

mgree commented Sep 25, 2021

I tried a run where I don't do anything in drop and only use destroy, and you can see that even there destroy isn't being called.

I haven't dug into fuser code yet, but could it be that the reference to my FS isn't living long enough, and it's getting dropped before the call to destroy gets made?

@mgree
Copy link
Contributor Author

mgree commented Sep 25, 2021

Oh, in fact: that fusermount error was the core issue. If you look at the ubuntu logs, it seems like FS is dropped right after the fusermount error. Apparently fuser is adding allow_other because I use autounmount.

Here's the /etc/fuse.conf GitHub CI is giving me (it's all defaults):

# /etc/fuse.conf - Configuration file for Filesystem in Userspace (FUSE)

# Set the maximum number of FUSE mounts allowed to non-root users.
# The default is 1000.
#mount_max = 1000

# Allow non-root users to specify the allow_other or allow_root mount options.
#user_allow_other

The new logic in 0009aa1 adds the allow_other option, which my config doesn't allow. Turning off autounmount seems to solve the problem.

Would you accept a PR that gave a user-facing warning (at warn level) when allow_other is automatically added due to autounmount?

@cberner
Copy link
Owner

cberner commented Sep 25, 2021

Yep, happy to a merge to PR to add that warning. Thanks for tracking it down!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants