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

Cancel regular unmount if filesystem is still in use #48

Merged
merged 8 commits into from
Sep 25, 2020

Conversation

infeo
Copy link
Member

@infeo infeo commented Sep 24, 2020

This PR closes #47.

It adds the method isNotSafeToUnmount() to FuseNioAdapter, to indicate wether the filesystem is in a state where the (internal) regular unmount method should be called.

To achieve this, the number of open files is checked and wether there is a pending (path locking) file system operation.

* 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
Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs tests

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the test can be simplified

if (threadnum % 2 == 0) {
try (PathLock lockThread = lockManager.createPathLock("/foo/bar/baz").tryForWriting()) {
counter.incrementAndGet();
Thread.sleep(10);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why sleep?
why 4 threads? one for read, one for write should be sufficient..?

try (PathLock lockThread = lockManager.createPathLock("/foo/bar/baz").tryForWriting()) {
counter.incrementAndGet();
Thread.sleep(10);
maxCounter.set(Math.max(counter.get(), maxCounter.get()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of maxCounter (btw this is not an atomic operation)

Copy link
Member

Codacy Here is an overview of what got changed by this pull request:

Coverage decreased per file
===========================
- src/main/java/org/cryptomator/frontend/fuse/locks/PathWLockImpl.java  -15
- src/main/java/org/cryptomator/frontend/fuse/OpenFileFactory.java  -2
- src/main/java/org/cryptomator/frontend/fuse/locks/PathLockBuilderImpl.java  -8
         

Complexity increasing per file
==============================
- src/main/java/org/cryptomator/frontend/fuse/locks/PathWLockImpl.java  2
- src/main/java/org/cryptomator/frontend/fuse/locks/PathRLockImpl.java  2
- src/main/java/org/cryptomator/frontend/fuse/locks/AlreadyLockedException.java  1
         

See the complete overview on Codacy

@infeo infeo changed the title Abort regular unmount if filesystem is still in use Cancel regular unmount if filesystem is still in use Sep 25, 2020
@infeo infeo merged commit 8e11e9e into develop Sep 25, 2020
@overheadhunter overheadhunter deleted the feature/#47-trackOpenFileChannels branch September 25, 2020 13:54
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 this pull request may close these issues.

Check for openFileHandles before regular unmount to cancel it
3 participants