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

Check for openFileHandles before regular unmount to cancel it #47

Closed
infeo opened this issue Jun 8, 2020 · 3 comments · Fixed by #48
Closed

Check for openFileHandles before regular unmount to cancel it #47

infeo opened this issue Jun 8, 2020 · 3 comments · Fixed by #48
Assignees
Milestone

Comments

@infeo
Copy link
Member

infeo commented Jun 8, 2020

If a file channel is still open when one executes the unmount method ( which in the end calls the linux helper command fusermount), the unmount fails with a CommandFailedException:

public void unmount() throws CommandFailedException {
if (!fuseAdapter.isMounted()) {
return;
}
Process proc = ProcessUtil.startAndWaitFor(getUnmountCommand(), 5, TimeUnit.SECONDS);
ProcessUtil.assertExitValue(proc, 0);
fuseAdapter.umount();
}

Nonetheless, with the call fuseAdapter.unmount() the actual filesystem is marked as not-mounted, make it impossible to recover from the failed regular unmount command:

public void umount() {
// this might be called multiple times: explicitly _and_ via a shutdown hook registered during mount() in AbstractFuseFS
if (mounted.compareAndSet(true, false)) {
LOG.debug("Marked file system adapter as unmounted.");
} else {
LOG.trace("File system adapter already unmounted.");
}
}

Additionally, the CommandFailedException for the fusermount only means that the operation did not succeeded, but does not indicate for what reason.

I propose to change the regular unmount to _ only_ execute fusermount and marking the filesystem as unmounted, if no internal file channels are open and otherwise throw a different exception. If the filesystem need to be unmoutned, the unmountForced() method can be used.

Ideas how to implement can be taken from cryptomator/dokany-nio-adapter@9a65c64

@overheadhunter
Copy link
Member

With 663ab4d, the mounted flag is no longer set atomically at the begin of execution but after successful unmount only. Note, there is no longer any protection against invoking unmount multiple times.

That said, there is not yet any "open file handle counter".

@infeo
Copy link
Member Author

infeo commented Aug 25, 2020

An easy way to implement the feature is to use the Read/Write-locks. These are applied before nearly every (important) method. hence only if currently no lock is accquired we give the green light to release.

@overheadhunter
Copy link
Member

An easy way to implement the feature is to use the Read/Write-locks. These are applied before nearly every (important) method. hence only if currently no lock is accquired we give the green light to release.

True. If it is possible to create an exclusive (i.e. write) lock for the root path, we can be sure, no other operation holds a lock.

Problem is, that currently there is no "root". Locking starts one level below root, since there has never been the need for file operations on root.

infeo added a commit that referenced this issue Sep 24, 2020
* adding isNotSafeToUnmount() method to FuseNioAdapter interface
* AbstractMount directly implements unmount() and unmountForced()
* AbstractMount has two additional _abstract_ methods for internal unmounting
* ReadOnlyAdapter implements isNotSafeToUnmount
* aborting unmount() if is not safe and throw CommandFailedException
@infeo infeo closed this as completed in #48 Sep 25, 2020
infeo pushed a commit that referenced this issue Sep 25, 2020
…nels

Cancel regular unmount if filesystem is still in use
@infeo infeo added this to the 1.2.5 milestone Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants