From b0ad86f16b84c7b9fcdc2b67fb8fce01dc185066 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 18 Nov 2020 14:07:34 +0100 Subject: [PATCH] Perform cleanup of tmp mount points dir only once. --- .../mountpoint/IrregularUnmountCleaner.java | 33 ++++++++++++++++--- .../TemporaryMountPointChooser.java | 26 ++++----------- 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java index a01e385d6..4e9866fda 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/IrregularUnmountCleaner.java @@ -1,20 +1,44 @@ package org.cryptomator.common.mountpoint; +import org.cryptomator.common.Environment; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.inject.Inject; +import javax.inject.Singleton; import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; +import java.util.Optional; +@Singleton class IrregularUnmountCleaner { public static Logger LOG = LoggerFactory.getLogger(IrregularUnmountCleaner.class); - public static void removeIrregularUnmountDebris(Path dirContainingMountPoints) { + private final Optional tmpMountPointDir; + private volatile boolean alreadyChecked = false; + + @Inject + public IrregularUnmountCleaner(Environment env) { + this.tmpMountPointDir = env.getMountPointsDir(); + } + + + public void clearIrregularUnmountDebrisIfNeeded() { + if (alreadyChecked || tmpMountPointDir.isEmpty()) { + return; //nuthin to do + } + if (Files.exists(tmpMountPointDir.get(), LinkOption.NOFOLLOW_LINKS)) { + clearIrregularUnmountDebris(tmpMountPointDir.get()); + } + alreadyChecked = true; + } + + private void clearIrregularUnmountDebris(Path dirContainingMountPoints) { IOException cleanupFailed = new IOException("Cleanup failed"); try { @@ -41,11 +65,12 @@ class IrregularUnmountCleaner { } } catch (IOException e) { LOG.warn("Unable to perform cleanup of mountpoint dir {}.", dirContainingMountPoints, e); + } finally { + alreadyChecked = true; } - } - private static void deleteEmptyDir(Path dir) throws IOException { + private void deleteEmptyDir(Path dir) throws IOException { assert Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS); try { Files.delete(dir); // attempt to delete dir non-recursively (will fail, if there are contents) @@ -54,7 +79,7 @@ class IrregularUnmountCleaner { } } - private static void deleteDeadLink(Path symlink) throws IOException { + private void deleteDeadLink(Path symlink) throws IOException { assert Files.isSymbolicLink(symlink); if (Files.notExists(symlink)) { // following link: target does not exist Files.delete(symlink); diff --git a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java index 0db2e7892..071ed7d1a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java +++ b/main/commons/src/main/java/org/cryptomator/common/mountpoint/TemporaryMountPointChooser.java @@ -1,6 +1,5 @@ package org.cryptomator.common.mountpoint; -import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Environment; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.vaults.Volume; @@ -11,11 +10,8 @@ import javax.inject.Inject; import java.io.File; import java.io.IOException; import java.nio.file.Files; -import java.nio.file.LinkOption; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; class TemporaryMountPointChooser implements MountPointChooser { @@ -24,12 +20,14 @@ class TemporaryMountPointChooser implements MountPointChooser { private final VaultSettings vaultSettings; private final Environment environment; + private final IrregularUnmountCleaner cleaner; private volatile boolean clearedDebris; @Inject - public TemporaryMountPointChooser(VaultSettings vaultSettings, Environment environment) { + public TemporaryMountPointChooser(VaultSettings vaultSettings, Environment environment, IrregularUnmountCleaner cleaner) { this.vaultSettings = vaultSettings; this.environment = environment; + this.cleaner = cleaner; } @Override @@ -43,23 +41,13 @@ class TemporaryMountPointChooser implements MountPointChooser { @Override public Optional chooseMountPoint(Volume caller) { - clearDebrisIfNeeded(); + assert environment.getMountPointsDir().isPresent(); + //clean leftovers of not-regularly unmounted vaults + //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 + cleaner.clearIrregularUnmountDebrisIfNeeded(); return this.environment.getMountPointsDir().map(this::choose); } - //clean leftovers of not-regularly unmounted vaults - //see https://github.com/cryptomator/cryptomator/issues/1013 and https://github.com/cryptomator/cryptomator/issues/1061 - private synchronized void clearDebrisIfNeeded() { - assert environment.getMountPointsDir().isPresent(); - if (clearedDebris) { - return; // already cleared - } - Path mountPointDir = environment.getMountPointsDir().get(); - if (Files.exists(mountPointDir, LinkOption.NOFOLLOW_LINKS)) { - IrregularUnmountCleaner.removeIrregularUnmountDebris(mountPointDir); - } - clearedDebris = true; - } private Path choose(Path parent) { String basename = this.vaultSettings.mountName().get();