From c05e00d32ad55f2975746a965274278f428b53d2 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 23 Mar 2021 12:37:36 +0100 Subject: [PATCH 01/30] Change volume interface to observe mounts --- .../java/org/cryptomator/common/vaults/DokanyVolume.java | 5 ++++- .../main/java/org/cryptomator/common/vaults/FuseVolume.java | 5 ++++- .../src/main/java/org/cryptomator/common/vaults/Volume.java | 3 ++- .../java/org/cryptomator/common/vaults/WebDavVolume.java | 5 ++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index a33dbd5ee..46aef447a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -13,6 +13,8 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutorService; public class DokanyVolume extends AbstractVolume { @@ -39,7 +41,7 @@ public class DokanyVolume extends AbstractVolume { } @Override - public void mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { + public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { this.mountPoint = determineMountPoint(); try { this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip()); @@ -49,6 +51,7 @@ public class DokanyVolume extends AbstractVolume { } throw new VolumeException("Unable to mount Filesystem", e); } + return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME } @Override diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index 5400f8ff2..b2b295e38 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -20,6 +20,8 @@ import javax.inject.Named; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.regex.Pattern; public class FuseVolume extends AbstractVolume { @@ -35,10 +37,11 @@ public class FuseVolume extends AbstractVolume { } @Override - public void mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { + public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { this.mountPoint = determineMountPoint(); mount(fs.getPath("/"), mountFlags); + return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME } private void mount(Path root, String mountFlags) throws VolumeException { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java index 74a307d5a..2ec05c6b0 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java @@ -7,6 +7,7 @@ import org.cryptomator.cryptofs.CryptoFileSystem; import java.io.IOException; import java.nio.file.Path; import java.util.Optional; +import java.util.concurrent.CompletionStage; import java.util.stream.Stream; /** @@ -32,7 +33,7 @@ public interface Volume { * @param fs * @throws IOException */ - void mount(CryptoFileSystem fs, String mountFlags) throws IOException, VolumeException, InvalidMountPointException; + CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws IOException, VolumeException, InvalidMountPointException; /** * Reveals the mounted volume. diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index 8b4f27fdb..53f336e97 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -17,6 +17,8 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.nio.file.Path; import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import java.util.function.Supplier; public class WebDavVolume implements Volume { @@ -41,9 +43,10 @@ public class WebDavVolume implements Volume { } @Override - public void mount(CryptoFileSystem fs, String mountFlags) throws VolumeException { + public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws VolumeException { startServlet(fs); mountServlet(); + return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME } private void startServlet(CryptoFileSystem fs){ From 17dc32bb79ce1bc7f3f3ee08ec5a0c732758bf4b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 23 Mar 2021 12:52:38 +0100 Subject: [PATCH 02/30] lock vault on external unmount --- .../main/java/org/cryptomator/common/vaults/Vault.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 98a6cf21b..5c87be3be 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -42,6 +42,7 @@ import java.util.EnumSet; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -139,7 +140,14 @@ public class Vault { cryptoFileSystem.set(fs); try { volume = volumeProvider.get(); - volume.mount(fs, getEffectiveMountFlags()); + volume.mount(fs, getEffectiveMountFlags()).handle((voit, throwable) -> { + destroyCryptoFileSystem(); + setState(VaultState.LOCKED); //TODO: possible race conditions of the vault state. Use Platform.runLater()? + if (throwable != null) { + LOG.warn("Unexpected unmount and lock of vault" + getDisplayName(), throwable); + } + return CompletableFuture.completedFuture(null); + }); } catch (Exception e) { destroyCryptoFileSystem(); throw e; From 629b6fb97d2501d9ffd1921fae7eb89a762137b3 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 23 Mar 2021 12:53:17 +0100 Subject: [PATCH 03/30] execute Service tasks on application thread --- .../main/java/org/cryptomator/common/vaults/VaultStats.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java index 46cf31991..90edd6604 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java @@ -52,13 +52,13 @@ public class VaultStats { } private void vaultStateChanged(@SuppressWarnings("unused") Observable observable) { - if (VaultState.UNLOCKED.equals(state.get())) { + if (VaultState.UNLOCKED == state.get()) { assert fs.get() != null; LOG.debug("start recording stats"); - updateService.restart(); + Platform.runLater(() -> updateService.restart()); } else { LOG.debug("stop recording stats"); - updateService.cancel(); + Platform.runLater(() -> updateService.cancel()); } } From cf7cbae567aaf9c9bb5f8eaefd4b3a7ba4788f8c Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 31 Mar 2021 11:31:45 +0200 Subject: [PATCH 04/30] Set dev branch back to SNAPSHOT version [ci skip] --- main/buildkit/pom.xml | 2 +- main/commons/pom.xml | 2 +- main/launcher/pom.xml | 2 +- main/pom.xml | 2 +- main/ui/pom.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/main/buildkit/pom.xml b/main/buildkit/pom.xml index 7ebb12112..4dde902b9 100644 --- a/main/buildkit/pom.xml +++ b/main/buildkit/pom.xml @@ -4,7 +4,7 @@ org.cryptomator main - 1.5.14 + 1.6.0-SNAPSHOT buildkit pom diff --git a/main/commons/pom.xml b/main/commons/pom.xml index e343999ce..28a42ec07 100644 --- a/main/commons/pom.xml +++ b/main/commons/pom.xml @@ -4,7 +4,7 @@ org.cryptomator main - 1.5.14 + 1.6.0-SNAPSHOT commons Cryptomator Commons diff --git a/main/launcher/pom.xml b/main/launcher/pom.xml index 64e142c23..a2559d145 100644 --- a/main/launcher/pom.xml +++ b/main/launcher/pom.xml @@ -4,7 +4,7 @@ org.cryptomator main - 1.5.14 + 1.6.0-SNAPSHOT launcher Cryptomator Launcher diff --git a/main/pom.xml b/main/pom.xml index a740a1427..6a99761b9 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.cryptomator main - 1.5.14 + 1.6.0-SNAPSHOT pom Cryptomator diff --git a/main/ui/pom.xml b/main/ui/pom.xml index 1332596e8..9e63fb4b0 100644 --- a/main/ui/pom.xml +++ b/main/ui/pom.xml @@ -4,7 +4,7 @@ org.cryptomator main - 1.5.14 + 1.6.0-SNAPSHOT ui Cryptomator GUI From fb1078b35bb8578ca7afa1791cef22b37db3bd6e Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 6 Apr 2021 10:05:32 +0200 Subject: [PATCH 05/30] bump to SNAPSHOT nio-adapter and refactor Volume.mount() method: * returns void * add onExitAction parameter * adjust classes --- .../common/vaults/DokanyVolume.java | 14 +++++------ .../cryptomator/common/vaults/FuseVolume.java | 23 ++++++++----------- .../org/cryptomator/common/vaults/Vault.java | 4 +--- .../org/cryptomator/common/vaults/Volume.java | 3 ++- .../common/vaults/WebDavVolume.java | 4 ++-- main/pom.xml | 4 ++-- 6 files changed, 22 insertions(+), 30 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index 46aef447a..e96ed1930 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -5,9 +5,9 @@ import org.cryptomator.common.mountpoint.MountPointChooser; import org.cryptomator.common.settings.VaultSettings; import org.cryptomator.common.settings.VolumeImpl; import org.cryptomator.cryptofs.CryptoFileSystem; +import org.cryptomator.frontend.dokany.DokanyMountFailedException; import org.cryptomator.frontend.dokany.Mount; import org.cryptomator.frontend.dokany.MountFactory; -import org.cryptomator.frontend.dokany.MountFailedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,6 +16,7 @@ import javax.inject.Named; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutorService; +import java.util.function.Consumer; public class DokanyVolume extends AbstractVolume { @@ -24,15 +25,13 @@ public class DokanyVolume extends AbstractVolume { private static final String FS_TYPE_NAME = "CryptomatorFS"; private final VaultSettings vaultSettings; - private final MountFactory mountFactory; private Mount mount; @Inject - public DokanyVolume(VaultSettings vaultSettings, ExecutorService executorService, @Named("orderedMountPointChoosers") Iterable choosers) { + public DokanyVolume(VaultSettings vaultSettings, @Named("orderedMountPointChoosers") Iterable choosers) { super(choosers); this.vaultSettings = vaultSettings; - this.mountFactory = new MountFactory(executorService); } @Override @@ -41,17 +40,16 @@ public class DokanyVolume extends AbstractVolume { } @Override - public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { + public void mount(CryptoFileSystem fs, String mountFlags, Consumer onExitAction) throws InvalidMountPointException, VolumeException { this.mountPoint = determineMountPoint(); try { - this.mount = mountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip()); - } catch (MountFailedException e) { + this.mount = MountFactory.mount(fs.getPath("/"), mountPoint, vaultSettings.mountName().get(), FS_TYPE_NAME, mountFlags.strip(), onExitAction); + } catch (DokanyMountFailedException e) { if (vaultSettings.getCustomMountPath().isPresent()) { LOG.warn("Failed to mount vault into {}. Is this directory currently accessed by another process (e.g. Windows Explorer)?", mountPoint); } throw new VolumeException("Unable to mount Filesystem", e); } - return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME } @Override diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java index f6e884a7a..a1579fdaf 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/FuseVolume.java @@ -6,8 +6,8 @@ import org.cryptomator.common.mountpoint.InvalidMountPointException; import org.cryptomator.common.mountpoint.MountPointChooser; import org.cryptomator.common.settings.VolumeImpl; import org.cryptomator.cryptofs.CryptoFileSystem; -import org.cryptomator.frontend.fuse.mount.CommandFailedException; import org.cryptomator.frontend.fuse.mount.EnvironmentVariables; +import org.cryptomator.frontend.fuse.mount.FuseMountException; import org.cryptomator.frontend.fuse.mount.FuseMountFactory; import org.cryptomator.frontend.fuse.mount.FuseNotSupportedException; import org.cryptomator.frontend.fuse.mount.Mount; @@ -20,8 +20,7 @@ import javax.inject.Named; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; +import java.util.function.Consumer; import java.util.regex.Pattern; public class FuseVolume extends AbstractVolume { @@ -37,14 +36,12 @@ public class FuseVolume extends AbstractVolume { } @Override - public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws InvalidMountPointException, VolumeException { + public void mount(CryptoFileSystem fs, String mountFlags, Consumer onExitAction) throws InvalidMountPointException, VolumeException { this.mountPoint = determineMountPoint(); - - mount(fs.getPath("/"), mountFlags); - return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME + mount(fs.getPath("/"), mountFlags, onExitAction); } - private void mount(Path root, String mountFlags) throws VolumeException { + private void mount(Path root, String mountFlags, Consumer onExitAction) throws VolumeException { try { Mounter mounter = FuseMountFactory.getMounter(); EnvironmentVariables envVars = EnvironmentVariables.create() // @@ -52,8 +49,8 @@ public class FuseVolume extends AbstractVolume { .withMountPoint(mountPoint) // .withFileNameTranscoder(mounter.defaultFileNameTranscoder()) // .build(); - this.mount = mounter.mount(root, envVars); - } catch (CommandFailedException | FuseNotSupportedException e) { + this.mount = mounter.mount(root, envVars, onExitAction); + } catch ( FuseMountException | FuseNotSupportedException e) { throw new VolumeException("Unable to mount Filesystem", e); } } @@ -94,8 +91,7 @@ public class FuseVolume extends AbstractVolume { public synchronized void unmountForced() throws VolumeException { try { mount.unmountForced(); - mount.close(); - } catch (CommandFailedException e) { + } catch (FuseMountException e) { throw new VolumeException(e); } cleanupMountPoint(); @@ -105,8 +101,7 @@ public class FuseVolume extends AbstractVolume { public synchronized void unmount() throws VolumeException { try { mount.unmount(); - mount.close(); - } catch (CommandFailedException e) { + } catch (FuseMountException e) { throw new VolumeException(e); } cleanupMountPoint(); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 5c87be3be..4a1295067 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -42,7 +42,6 @@ import java.util.EnumSet; import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -140,13 +139,12 @@ public class Vault { cryptoFileSystem.set(fs); try { volume = volumeProvider.get(); - volume.mount(fs, getEffectiveMountFlags()).handle((voit, throwable) -> { + volume.mount(fs, getEffectiveMountFlags(), throwable -> { destroyCryptoFileSystem(); setState(VaultState.LOCKED); //TODO: possible race conditions of the vault state. Use Platform.runLater()? if (throwable != null) { LOG.warn("Unexpected unmount and lock of vault" + getDisplayName(), throwable); } - return CompletableFuture.completedFuture(null); }); } catch (Exception e) { destroyCryptoFileSystem(); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java index 2ec05c6b0..f608122bf 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Volume.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.Optional; import java.util.concurrent.CompletionStage; +import java.util.function.Consumer; import java.util.stream.Stream; /** @@ -33,7 +34,7 @@ public interface Volume { * @param fs * @throws IOException */ - CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws IOException, VolumeException, InvalidMountPointException; + void mount(CryptoFileSystem fs, String mountFlags, Consumer onExitAction) throws IOException, VolumeException, InvalidMountPointException; /** * Reveals the mounted volume. diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index 53f336e97..352cfb489 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -19,6 +19,7 @@ import java.nio.file.Path; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import java.util.function.Consumer; import java.util.function.Supplier; public class WebDavVolume implements Volume { @@ -43,10 +44,9 @@ public class WebDavVolume implements Volume { } @Override - public CompletionStage mount(CryptoFileSystem fs, String mountFlags) throws VolumeException { + public void mount(CryptoFileSystem fs, String mountFlags, Consumer onExitAction) throws VolumeException { startServlet(fs); mountServlet(); - return CompletableFuture.failedFuture(new IllegalStateException("THOU SHOULD NOT PASS")); //FIXME } private void startServlet(CryptoFileSystem fs){ diff --git a/main/pom.xml b/main/pom.xml index 6a99761b9..51ed64413 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -30,8 +30,8 @@ 1.0.0-beta2 1.0.0-beta2 1.0.0-beta1 - 1.3.0 - 1.2.4 + 1.4.0-SNAPSHOT + 1.3.0-SNAPSHOT 1.2.0 From beba6490c3a8f932e7a78f51c32465b6033b225b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 6 Apr 2021 13:26:58 +0200 Subject: [PATCH 06/30] Add locking mechanism to change the vault state t --- .../org/cryptomator/common/vaults/Vault.java | 34 ++++++++++++++++--- .../common/vaults/VaultListManager.java | 8 +++-- .../cryptomator/ui/common/VaultService.java | 17 ++++++++-- .../org/cryptomator/ui/lock/LockWorkflow.java | 14 +++++--- .../ui/migration/MigrationRunController.java | 16 +++++---- .../cryptomator/ui/unlock/UnlockWorkflow.java | 14 +++++--- 6 files changed, 78 insertions(+), 25 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 4a1295067..584c79456 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -27,6 +27,7 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Provider; +import javafx.application.Platform; import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanBinding; @@ -43,6 +44,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.StampedLock; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -52,6 +54,8 @@ public class Vault { private static final Logger LOG = LoggerFactory.getLogger(Vault.class); private static final Path HOME_DIR = Paths.get(SystemUtils.USER_HOME); + private final StampedLock stateLock; + private final VaultSettings vaultSettings; private final Provider volumeProvider; private final StringBinding defaultMountFlags; @@ -93,6 +97,8 @@ public class Vault { this.accessPoint = Bindings.createStringBinding(this::getAccessPoint, state); this.accessPointPresent = this.accessPoint.isNotEmpty(); this.showingStats = new SimpleBooleanProperty(false); + + this.stateLock = new StampedLock(); } // ****************************************************************************** @@ -141,9 +147,13 @@ public class Vault { volume = volumeProvider.get(); volume.mount(fs, getEffectiveMountFlags(), throwable -> { destroyCryptoFileSystem(); - setState(VaultState.LOCKED); //TODO: possible race conditions of the vault state. Use Platform.runLater()? + new Thread(() -> { //TODO: maybe use the executor service + long stamp = stateLock.writeLock(); + setState(VaultState.LOCKED, stamp); + stateLock.unlock(stamp); + }).start(); if (throwable != null) { - LOG.warn("Unexpected unmount and lock of vault" + getDisplayName(), throwable); + LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); } }); } catch (Exception e) { @@ -180,8 +190,24 @@ public class Vault { return state.get(); } - public void setState(VaultState value) { - state.setValue(value); + public long lockVaultState() { + return stateLock.writeLock(); + } + + public void unlockVaultState(long stamp) { + stateLock.unlock(stamp); + } + + public void setState(VaultState value, long stamp) { + if (stateLock.isWriteLockStamp(stamp)) { + if (Platform.isFxApplicationThread()) { + state.setValue(value); + } else { + Platform.runLater(() -> state.setValue(value)); + } + } else { + throw new IllegalCallerException("Stamp is not a valid write lock."); + } } public ObjectProperty lastKnownExceptionProperty() { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index b7c10a4d4..da9eee66e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -25,7 +25,6 @@ import java.nio.file.Path; import java.util.Collection; import java.util.Optional; import java.util.ResourceBundle; -import java.util.stream.Collectors; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -108,15 +107,18 @@ public class VaultListManager { VaultState previousState = vault.getState(); return switch (previousState) { case LOCKED, NEEDS_MIGRATION, MISSING -> { + long stamp = vault.lockVaultState(); try { VaultState determinedState = determineVaultState(vault.getPath()); - vault.setState(determinedState); + vault.setState(determinedState, stamp); yield determinedState; } catch (IOException e) { LOG.warn("Failed to determine vault state for " + vault.getPath(), e); - vault.setState(VaultState.ERROR); + vault.setState(VaultState.ERROR, stamp); vault.setLastKnownException(e); yield VaultState.ERROR; + } finally { + vault.unlockVaultState(stamp); } } case ERROR, UNLOCKED, PROCESSING -> previousState; diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java index 57393b858..d96bd6ae2 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java @@ -163,6 +163,8 @@ public class VaultService { private final Vault vault; private final boolean forced; + private volatile long stamp; + /** * @param vault The vault to lock * @param forced Whether to attempt a forced lock @@ -176,23 +178,32 @@ public class VaultService { @Override protected Vault call() throws Volume.VolumeException { + this.stamp = vault.lockVaultState(); vault.lock(forced); return vault; } @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING); + vault.setState(VaultState.PROCESSING, stamp); } @Override protected void succeeded() { - vault.setState(VaultState.LOCKED); + vault.setState(VaultState.LOCKED, stamp); + vault.unlockVaultState(stamp); } @Override protected void failed() { - vault.setState(VaultState.UNLOCKED); + vault.setState(VaultState.UNLOCKED, stamp); + vault.unlockVaultState(stamp); + } + + @Override + protected void cancelled() { + vault.setState(VaultState.UNLOCKED, stamp); + vault.unlockVaultState(stamp); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index acb6d1355..e7f0eb025 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -36,6 +36,8 @@ public class LockWorkflow extends Task { private final Lazy lockForcedScene; private final Lazy lockFailedScene; + private volatile long stamp; + @Inject public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene) { this.lockWindow = lockWindow; @@ -47,6 +49,7 @@ public class LockWorkflow extends Task { @Override protected Void call() throws Volume.VolumeException, InterruptedException { + this.stamp = vault.lockVaultState(); try { vault.lock(false); } catch (Volume.VolumeException e) { @@ -79,19 +82,21 @@ public class LockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING); + vault.setState(VaultState.PROCESSING, stamp); } @Override protected void succeeded() { LOG.info("Lock of {} succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED); + vault.setState(VaultState.LOCKED, stamp); + vault.unlockVaultState(stamp); } @Override protected void failed() { LOG.warn("Failed to lock {}.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED); + vault.setState(VaultState.UNLOCKED, stamp); + vault.unlockVaultState(stamp); lockWindow.setScene(lockFailedScene.get()); lockWindow.show(); } @@ -99,7 +104,8 @@ public class LockWorkflow extends Task { @Override protected void cancelled() { LOG.debug("Lock of {} canceled.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED); + vault.setState(VaultState.UNLOCKED, stamp); + vault.unlockVaultState(stamp); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java index 830e15819..75caddf5d 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java @@ -101,7 +101,8 @@ public class MigrationRunController implements FxController { public void migrate() { LOG.info("Migrating vault {}", vault.getPath()); CharSequence password = passwordField.getCharacters(); - vault.setState(VaultState.PROCESSING); + long stamp = vault.lockVaultState(); + vault.setState(VaultState.PROCESSING, stamp); passwordField.setDisable(true); ScheduledFuture progressSyncTask = scheduler.scheduleAtFixedRate(() -> { Platform.runLater(() -> { @@ -115,10 +116,10 @@ public class MigrationRunController implements FxController { }).onSuccess(needsAnotherMigration -> { if (needsAnotherMigration) { LOG.info("Migration of '{}' succeeded, but another migration is required.", vault.getDisplayName()); - vault.setState(VaultState.NEEDS_MIGRATION); + vault.setState(VaultState.NEEDS_MIGRATION, stamp); } else { LOG.info("Migration of '{}' succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED); + vault.setState(VaultState.LOCKED, stamp); passwordField.wipe(); window.setScene(successScene.get()); } @@ -127,22 +128,23 @@ public class MigrationRunController implements FxController { passwordField.setDisable(false); passwordField.selectAll(); passwordField.requestFocus(); - vault.setState(VaultState.NEEDS_MIGRATION); + vault.setState(VaultState.NEEDS_MIGRATION, stamp); }).onError(FileSystemCapabilityChecker.MissingCapabilityException.class, e -> { LOG.error("Underlying file system not supported.", e); - vault.setState(VaultState.NEEDS_MIGRATION); + vault.setState(VaultState.NEEDS_MIGRATION, stamp); missingCapability.set(e.getMissingCapability()); window.setScene(capabilityErrorScene.get()); }).onError(FileNameTooLongException.class, e -> { LOG.error("Migration failed because the underlying file system does not support long filenames.", e); - vault.setState(VaultState.NEEDS_MIGRATION); + vault.setState(VaultState.NEEDS_MIGRATION, stamp); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); window.setScene(impossibleScene.get()); }).onError(Exception.class, e -> { // including RuntimeExceptions LOG.error("Migration failed for technical reasons.", e); - vault.setState(VaultState.NEEDS_MIGRATION); + vault.setState(VaultState.NEEDS_MIGRATION, stamp); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); }).andFinally(() -> { + vault.unlockVaultState(stamp); passwordField.setDisable(false); progressSyncTask.cancel(true); }).runOnce(executor); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index c1b5fbd45..36d34c170 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -59,6 +59,8 @@ public class UnlockWorkflow extends Task { private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; + private volatile long stamp; + @Inject UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { this.window = window; @@ -87,6 +89,7 @@ public class UnlockWorkflow extends Task { @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { try { + this.stamp = vault.lockVaultState(); if (attemptUnlock()) { handleSuccess(); return true; @@ -207,22 +210,25 @@ public class UnlockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING); + vault.setState(VaultState.PROCESSING, stamp); } @Override protected void succeeded() { - vault.setState(VaultState.UNLOCKED); + vault.setState(VaultState.UNLOCKED, stamp); + vault.unlockVaultState(stamp); } @Override protected void failed() { - vault.setState(VaultState.LOCKED); + vault.setState(VaultState.LOCKED, stamp); + vault.unlockVaultState(stamp); } @Override protected void cancelled() { - vault.setState(VaultState.LOCKED); + vault.setState(VaultState.LOCKED, stamp); + vault.unlockVaultState(stamp); } } From c306d8df04c343eaeb265ba3582c524c1b3c4db7 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 8 Apr 2021 11:23:57 +0200 Subject: [PATCH 07/30] alternative thread-safe vault state without requiring explicit synchronization --- .../common/vaults/DokanyVolume.java | 3 - .../org/cryptomator/common/vaults/Vault.java | 57 +++------ .../common/vaults/VaultComponent.java | 2 +- .../common/vaults/VaultListManager.java | 28 +++-- .../common/vaults/VaultModule.java | 6 - .../cryptomator/common/vaults/VaultState.java | 110 +++++++++++++----- .../cryptomator/common/vaults/VaultStats.java | 6 +- .../cryptomator/ui/common/VaultService.java | 14 +-- .../ui/launcher/AppLifecycleListener.java | 4 +- .../org/cryptomator/ui/lock/LockWorkflow.java | 14 +-- .../ui/mainwindow/VaultDetailController.java | 3 +- .../mainwindow/VaultListCellController.java | 3 +- .../VaultListContextMenuController.java | 11 +- .../ui/migration/MigrationRunController.java | 22 ++-- .../ui/stats/VaultStatisticsModule.java | 4 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 17 +-- 16 files changed, 150 insertions(+), 154 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java index e96ed1930..c998761d0 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/DokanyVolume.java @@ -13,9 +13,6 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; -import java.util.concurrent.ExecutorService; import java.util.function.Consumer; public class DokanyVolume extends AbstractVolume { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 584c79456..39cf6d2fd 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -27,7 +27,6 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Named; import javax.inject.Provider; -import javafx.application.Platform; import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.binding.BooleanBinding; @@ -44,7 +43,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.StampedLock; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -54,13 +52,11 @@ public class Vault { private static final Logger LOG = LoggerFactory.getLogger(Vault.class); private static final Path HOME_DIR = Paths.get(SystemUtils.USER_HOME); - private final StampedLock stateLock; - private final VaultSettings vaultSettings; private final Provider volumeProvider; private final StringBinding defaultMountFlags; private final AtomicReference cryptoFileSystem; - private final ObjectProperty state; + private final VaultState state; private final ObjectProperty lastKnownException; private final VaultStats stats; private final StringBinding displayName; @@ -78,7 +74,7 @@ public class Vault { private volatile Volume volume; @Inject - Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, ObjectProperty state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { + Vault(VaultSettings vaultSettings, Provider volumeProvider, @DefaultMountFlags StringBinding defaultMountFlags, AtomicReference cryptoFileSystem, VaultState state, @Named("lastKnownException") ObjectProperty lastKnownException, VaultStats stats) { this.vaultSettings = vaultSettings; this.volumeProvider = volumeProvider; this.defaultMountFlags = defaultMountFlags; @@ -97,8 +93,6 @@ public class Vault { this.accessPoint = Bindings.createStringBinding(this::getAccessPoint, state); this.accessPointPresent = this.accessPoint.isNotEmpty(); this.showingStats = new SimpleBooleanProperty(false); - - this.stateLock = new StampedLock(); } // ****************************************************************************** @@ -146,12 +140,9 @@ public class Vault { try { volume = volumeProvider.get(); volume.mount(fs, getEffectiveMountFlags(), throwable -> { + LOG.info("Unmounted vault '{}'", getDisplayName()); destroyCryptoFileSystem(); - new Thread(() -> { //TODO: maybe use the executor service - long stamp = stateLock.writeLock(); - setState(VaultState.LOCKED, stamp); - stateLock.unlock(stamp); - }).start(); + state.set(VaultState.Value.LOCKED); if (throwable != null) { LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); } @@ -182,32 +173,12 @@ public class Vault { // Observable Properties // ******************************************************************************* - public ObjectProperty stateProperty() { + public VaultState stateProperty() { return state; } - public VaultState getState() { - return state.get(); - } - - public long lockVaultState() { - return stateLock.writeLock(); - } - - public void unlockVaultState(long stamp) { - stateLock.unlock(stamp); - } - - public void setState(VaultState value, long stamp) { - if (stateLock.isWriteLockStamp(stamp)) { - if (Platform.isFxApplicationThread()) { - state.setValue(value); - } else { - Platform.runLater(() -> state.setValue(value)); - } - } else { - throw new IllegalCallerException("Stamp is not a valid write lock."); - } + public VaultState.Value getState() { + return state.getValue(); } public ObjectProperty lastKnownExceptionProperty() { @@ -227,7 +198,7 @@ public class Vault { } public boolean isLocked() { - return state.get() == VaultState.LOCKED; + return state.get() == VaultState.Value.LOCKED; } public BooleanBinding processingProperty() { @@ -235,7 +206,7 @@ public class Vault { } public boolean isProcessing() { - return state.get() == VaultState.PROCESSING; + return state.get() == VaultState.Value.PROCESSING; } public BooleanBinding unlockedProperty() { @@ -243,7 +214,7 @@ public class Vault { } public boolean isUnlocked() { - return state.get() == VaultState.UNLOCKED; + return state.get() == VaultState.Value.UNLOCKED; } public BooleanBinding missingProperty() { @@ -251,7 +222,7 @@ public class Vault { } public boolean isMissing() { - return state.get() == VaultState.MISSING; + return state.get() == VaultState.Value.MISSING; } public BooleanBinding needsMigrationProperty() { @@ -259,7 +230,7 @@ public class Vault { } public boolean isNeedsMigration() { - return state.get() == VaultState.NEEDS_MIGRATION; + return state.get() == VaultState.Value.NEEDS_MIGRATION; } public BooleanBinding unknownErrorProperty() { @@ -267,7 +238,7 @@ public class Vault { } public boolean isUnknownError() { - return state.get() == VaultState.ERROR; + return state.get() == VaultState.Value.ERROR; } public StringBinding displayNameProperty() { @@ -283,7 +254,7 @@ public class Vault { } public String getAccessPoint() { - if (state.get() == VaultState.UNLOCKED) { + if (state.getValue() == VaultState.Value.UNLOCKED) { assert volume != null; return volume.getMountPoint().orElse(Path.of("")).toString(); } else { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java index debfab3ed..47be62520 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultComponent.java @@ -26,7 +26,7 @@ public interface VaultComponent { Builder vaultSettings(VaultSettings vaultSettings); @BindsInstance - Builder initialVaultState(VaultState vaultState); + Builder initialVaultState(VaultState.Value vaultState); @BindsInstance Builder initialErrorCause(@Nullable @Named("lastKnownException") Exception initialErrorCause); diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java index da9eee66e..4b8ca71e8 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultListManager.java @@ -93,45 +93,43 @@ public class VaultListManager { private Vault create(VaultSettings vaultSettings) { VaultComponent.Builder compBuilder = vaultComponentBuilder.vaultSettings(vaultSettings); try { - VaultState vaultState = determineVaultState(vaultSettings.path().get()); + VaultState.Value vaultState = determineVaultState(vaultSettings.path().get()); compBuilder.initialVaultState(vaultState); } catch (IOException e) { LOG.warn("Failed to determine vault state for " + vaultSettings.path().get(), e); - compBuilder.initialVaultState(VaultState.ERROR); + compBuilder.initialVaultState(VaultState.Value.ERROR); compBuilder.initialErrorCause(e); } return compBuilder.build().vault(); } - public static VaultState redetermineVaultState(Vault vault) { - VaultState previousState = vault.getState(); + public static VaultState.Value redetermineVaultState(Vault vault) { + VaultState state = vault.stateProperty(); + VaultState.Value previousState = state.getValue(); return switch (previousState) { case LOCKED, NEEDS_MIGRATION, MISSING -> { - long stamp = vault.lockVaultState(); try { - VaultState determinedState = determineVaultState(vault.getPath()); - vault.setState(determinedState, stamp); + VaultState.Value determinedState = determineVaultState(vault.getPath()); + state.set(determinedState); yield determinedState; } catch (IOException e) { LOG.warn("Failed to determine vault state for " + vault.getPath(), e); - vault.setState(VaultState.ERROR, stamp); + state.set(VaultState.Value.ERROR); vault.setLastKnownException(e); - yield VaultState.ERROR; - } finally { - vault.unlockVaultState(stamp); + yield VaultState.Value.ERROR; } } case ERROR, UNLOCKED, PROCESSING -> previousState; }; } - private static VaultState determineVaultState(Path pathToVault) throws IOException { + private static VaultState.Value determineVaultState(Path pathToVault) throws IOException { if (!CryptoFileSystemProvider.containsVault(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.MISSING; + return VaultState.Value.MISSING; } else if (Migrators.get().needsMigration(pathToVault, MASTERKEY_FILENAME)) { - return VaultState.NEEDS_MIGRATION; + return VaultState.Value.NEEDS_MIGRATION; } else { - return VaultState.LOCKED; + return VaultState.Value.LOCKED; } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java index 56a2814cf..b6e070310 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultModule.java @@ -40,12 +40,6 @@ public class VaultModule { return new AtomicReference<>(); } - @Provides - @PerVault - public ObjectProperty provideVaultState(VaultState initialState) { - return new SimpleObjectProperty<>(initialState); - } - @Provides @Named("lastKnownException") @PerVault diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java index fa5e6c295..8966e6f65 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java @@ -1,34 +1,92 @@ package org.cryptomator.common.vaults; -public enum VaultState { - /** - * No vault found at the provided path - */ - MISSING, +import com.google.common.base.Preconditions; + +import javax.inject.Inject; +import javafx.application.Platform; +import javafx.beans.value.ObservableObjectValue; +import javafx.beans.value.ObservableValueBase; +import java.util.concurrent.atomic.AtomicReference; + +@PerVault +public class VaultState extends ObservableValueBase implements ObservableObjectValue { + + public enum Value { + /** + * No vault found at the provided path + */ + MISSING, + + /** + * Vault requires migration to a newer vault format + */ + NEEDS_MIGRATION, + + /** + * Vault ready to be unlocked + */ + LOCKED, + + /** + * Vault in transition between two other states + */ + PROCESSING, + + /** + * Vault is unlocked + */ + UNLOCKED, + + /** + * Unknown state due to preceeding unrecoverable exceptions. + */ + ERROR; + } + + private final AtomicReference value; + + @Inject + public VaultState(VaultState.Value initialValue){ + this.value = new AtomicReference<>(initialValue); + } + + @Override + public Value get() { + return getValue(); + } + + @Override + public Value getValue() { + return value.get(); + } /** - * Vault requires migration to a newer vault format + * Transitions from fromState to toState. + * @param fromState Previous state + * @param toState New state + * @return true if successful */ - NEEDS_MIGRATION, + public boolean transition(Value fromState, Value toState) { + Preconditions.checkArgument(fromState != toState, "fromState must be different than toState"); + boolean success = value.compareAndSet(fromState, toState); + if (success) { + fireValueChangedEvent(); + } + return success; + } - /** - * Vault ready to be unlocked - */ - LOCKED, - - /** - * Vault in transition between two other states - */ - PROCESSING, - - /** - * Vault is unlocked - */ - UNLOCKED, - - /** - * Unknown state due to preceeding unrecoverable exceptions. - */ - ERROR; + public void set(Value newState) { + var oldState = value.getAndSet(newState); + if (oldState != newState) { + fireValueChangedEvent(); + } + } + protected void fireValueChangedEvent() { + if (Platform.isFxApplicationThread()) { + super.fireValueChangedEvent(); + } else { + Platform.runLater(super::fireValueChangedEvent); + } + } } diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java index 90edd6604..6dc86e8be 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultStats.java @@ -26,7 +26,7 @@ public class VaultStats { private static final Logger LOG = LoggerFactory.getLogger(VaultStats.class); private final AtomicReference fs; - private final ObjectProperty state; + private final VaultState state; private final ScheduledService> updateService; private final LongProperty bytesPerSecondRead = new SimpleLongProperty(); private final LongProperty bytesPerSecondWritten = new SimpleLongProperty(); @@ -41,7 +41,7 @@ public class VaultStats { private final LongProperty filesWritten = new SimpleLongProperty(); @Inject - VaultStats(AtomicReference fs, ObjectProperty state, ExecutorService executor) { + VaultStats(AtomicReference fs, VaultState state, ExecutorService executor) { this.fs = fs; this.state = state; this.updateService = new UpdateStatsService(); @@ -52,7 +52,7 @@ public class VaultStats { } private void vaultStateChanged(@SuppressWarnings("unused") Observable observable) { - if (VaultState.UNLOCKED == state.get()) { + if (VaultState.Value.UNLOCKED == state.get()) { assert fs.get() != null; LOG.debug("start recording stats"); Platform.runLater(() -> updateService.restart()); diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java index d96bd6ae2..6555904e5 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java @@ -163,8 +163,6 @@ public class VaultService { private final Vault vault; private final boolean forced; - private volatile long stamp; - /** * @param vault The vault to lock * @param forced Whether to attempt a forced lock @@ -178,32 +176,28 @@ public class VaultService { @Override protected Vault call() throws Volume.VolumeException { - this.stamp = vault.lockVaultState(); vault.lock(forced); return vault; } @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void failed() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } @Override protected void cancelled() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java index 646c3ce6b..5e2ed383a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java @@ -24,11 +24,13 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import static org.cryptomator.common.vaults.VaultState.Value.*; + @Singleton public class AppLifecycleListener { private static final Logger LOG = LoggerFactory.getLogger(AppLifecycleListener.class); - public static final Set STATES_ALLOWING_TERMINATION = EnumSet.of(VaultState.LOCKED, VaultState.NEEDS_MIGRATION, VaultState.MISSING, VaultState.ERROR); + public static final Set STATES_ALLOWING_TERMINATION = EnumSet.of(LOCKED, NEEDS_MIGRATION, MISSING, ERROR); private final FxApplicationStarter fxApplicationStarter; private final CountDownLatch shutdownLatch; diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index e7f0eb025..673b74641 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -36,8 +36,6 @@ public class LockWorkflow extends Task { private final Lazy lockForcedScene; private final Lazy lockFailedScene; - private volatile long stamp; - @Inject public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene) { this.lockWindow = lockWindow; @@ -49,7 +47,6 @@ public class LockWorkflow extends Task { @Override protected Void call() throws Volume.VolumeException, InterruptedException { - this.stamp = vault.lockVaultState(); try { vault.lock(false); } catch (Volume.VolumeException e) { @@ -82,21 +79,19 @@ public class LockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { LOG.info("Lock of {} succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void failed() { LOG.warn("Failed to lock {}.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); lockWindow.setScene(lockFailedScene.get()); lockWindow.show(); } @@ -104,8 +99,7 @@ public class LockWorkflow extends Task { @Override protected void cancelled() { LOG.debug("Lock of {} canceled.", vault.getDisplayName()); - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java index 40df6b7c5..7b5c4c7c9 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailController.java @@ -32,7 +32,8 @@ public class VaultDetailController implements FxController { this.anyVaultSelected = vault.isNotNull(); } - private FontAwesome5Icon getGlyphForVaultState(VaultState state) { + // TODO deduplicate w/ VaultListCellController + private FontAwesome5Icon getGlyphForVaultState(VaultState.Value state) { if (state != null) { return switch (state) { case LOCKED -> FontAwesome5Icon.LOCK; diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java index abc287d6e..f18369e7a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListCellController.java @@ -24,7 +24,8 @@ public class VaultListCellController implements FxController { .map(this::getGlyphForVaultState); } - private FontAwesome5Icon getGlyphForVaultState(VaultState state) { + // TODO deduplicate w/ VaultDetailController + private FontAwesome5Icon getGlyphForVaultState(VaultState.Value state) { if (state != null) { return switch (state) { case LOCKED -> FontAwesome5Icon.LOCK; diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java index b0866d80f..145618fa8 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultListContextMenuController.java @@ -14,19 +14,13 @@ import org.cryptomator.ui.vaultoptions.VaultOptionsComponent; import javax.inject.Inject; import javafx.beans.binding.Binding; -import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; import javafx.fxml.FXML; import javafx.stage.Stage; -import java.util.Arrays; import java.util.EnumSet; import java.util.Optional; -import static org.cryptomator.common.vaults.VaultState.ERROR; -import static org.cryptomator.common.vaults.VaultState.LOCKED; -import static org.cryptomator.common.vaults.VaultState.MISSING; -import static org.cryptomator.common.vaults.VaultState.NEEDS_MIGRATION; -import static org.cryptomator.common.vaults.VaultState.UNLOCKED; +import static org.cryptomator.common.vaults.VaultState.Value.*; @MainWindowScoped public class VaultListContextMenuController implements FxController { @@ -37,7 +31,7 @@ public class VaultListContextMenuController implements FxController { private final KeychainManager keychain; private final RemoveVaultComponent.Builder removeVault; private final VaultOptionsComponent.Builder vaultOptionsWindow; - private final OptionalBinding selectedVaultState; + private final OptionalBinding selectedVaultState; private final Binding selectedVaultPassphraseStored; private final Binding selectedVaultRemovable; private final Binding selectedVaultUnlockable; @@ -57,7 +51,6 @@ public class VaultListContextMenuController implements FxController { this.selectedVaultRemovable = selectedVaultState.map(EnumSet.of(LOCKED, MISSING, ERROR, NEEDS_MIGRATION)::contains).orElse(false); this.selectedVaultUnlockable = selectedVaultState.map(LOCKED::equals).orElse(false); this.selectedVaultLockable = selectedVaultState.map(UNLOCKED::equals).orElse(false); - } private boolean isPasswordStored(Vault vault) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java index 75caddf5d..0fc66c992 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java @@ -26,6 +26,7 @@ import javax.inject.Named; import javafx.application.Platform; import javafx.beans.binding.Bindings; import javafx.beans.binding.ObjectBinding; +import javafx.beans.binding.ObjectExpression; import javafx.beans.property.BooleanProperty; import javafx.beans.property.DoubleProperty; import javafx.beans.property.ObjectProperty; @@ -89,7 +90,10 @@ public class MigrationRunController implements FxController { if (keychain.isSupported()) { loadStoredPassword(); } - migrationButtonDisabled.bind(vault.stateProperty().isNotEqualTo(VaultState.NEEDS_MIGRATION).or(passwordField.textProperty().isEmpty())); + + migrationButtonDisabled.bind(ObjectExpression.objectExpression(vault.stateProperty()) + .isNotEqualTo(VaultState.Value.NEEDS_MIGRATION) + .or(passwordField.textProperty().isEmpty())); } @FXML @@ -101,8 +105,7 @@ public class MigrationRunController implements FxController { public void migrate() { LOG.info("Migrating vault {}", vault.getPath()); CharSequence password = passwordField.getCharacters(); - long stamp = vault.lockVaultState(); - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.NEEDS_MIGRATION, VaultState.Value.PROCESSING); passwordField.setDisable(true); ScheduledFuture progressSyncTask = scheduler.scheduleAtFixedRate(() -> { Platform.runLater(() -> { @@ -116,10 +119,10 @@ public class MigrationRunController implements FxController { }).onSuccess(needsAnotherMigration -> { if (needsAnotherMigration) { LOG.info("Migration of '{}' succeeded, but another migration is required.", vault.getDisplayName()); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); } else { LOG.info("Migration of '{}' succeeded.", vault.getDisplayName()); - vault.setState(VaultState.LOCKED, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); passwordField.wipe(); window.setScene(successScene.get()); } @@ -128,23 +131,22 @@ public class MigrationRunController implements FxController { passwordField.setDisable(false); passwordField.selectAll(); passwordField.requestFocus(); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); }).onError(FileSystemCapabilityChecker.MissingCapabilityException.class, e -> { LOG.error("Underlying file system not supported.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); missingCapability.set(e.getMissingCapability()); window.setScene(capabilityErrorScene.get()); }).onError(FileNameTooLongException.class, e -> { LOG.error("Migration failed because the underlying file system does not support long filenames.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); window.setScene(impossibleScene.get()); }).onError(Exception.class, e -> { // including RuntimeExceptions LOG.error("Migration failed for technical reasons.", e); - vault.setState(VaultState.NEEDS_MIGRATION, stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.NEEDS_MIGRATION); errorComponent.cause(e).window(window).returnToScene(startScene.get()).build().showErrorScene(); }).andFinally(() -> { - vault.unlockVaultState(stamp); passwordField.setDisable(false); progressSyncTask.cancel(true); }).runOnce(executor); diff --git a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java index 803de314c..6f195274c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsModule.java @@ -43,8 +43,8 @@ abstract class VaultStatisticsModule { var weakStage = new WeakReference<>(stage); vault.stateProperty().addListener(new ChangeListener<>() { @Override - public void changed(ObservableValue observable, VaultState oldValue, VaultState newValue) { - if (newValue != VaultState.UNLOCKED) { + public void changed(ObservableValue observable, VaultState.Value oldValue, VaultState.Value newValue) { + if (newValue != VaultState.Value.UNLOCKED) { Stage stage = weakStage.get(); if (stage != null) { stage.hide(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index 36d34c170..08ae1fd4c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -59,8 +59,6 @@ public class UnlockWorkflow extends Task { private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; - private volatile long stamp; - @Inject UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { this.window = window; @@ -89,7 +87,6 @@ public class UnlockWorkflow extends Task { @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { try { - this.stamp = vault.lockVaultState(); if (attemptUnlock()) { handleSuccess(); return true; @@ -176,15 +173,12 @@ public class UnlockWorkflow extends Task { LOG.error("Unlock failed. Mountpoint doesn't exist (needs to be a folder): {}", cause.getMessage()); } showInvalidMountPointScene(); - return; } else if (cause instanceof FileAlreadyExistsException) { LOG.error("Unlock failed. Mountpoint already exists: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else if (cause instanceof DirectoryNotEmptyException) { LOG.error("Unlock failed. Mountpoint not an empty directory: {}", cause.getMessage()); showInvalidMountPointScene(); - return; } else { handleGenericError(impExc); } @@ -210,25 +204,22 @@ public class UnlockWorkflow extends Task { @Override protected void scheduled() { - vault.setState(VaultState.PROCESSING, stamp); + vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING); } @Override protected void succeeded() { - vault.setState(VaultState.UNLOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } @Override protected void failed() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void cancelled() { - vault.setState(VaultState.LOCKED, stamp); - vault.unlockVaultState(stamp); + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } } From 8447f105b0ed4d390286abd18330ef7eba5c0dd4 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 8 Apr 2021 13:08:05 +0200 Subject: [PATCH 08/30] rename instance variables --- .../org/cryptomator/ui/fxapp/FxApplication.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java index 917e704fc..d33b1171c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java @@ -44,8 +44,8 @@ public class FxApplication extends Application { private final Lazy mainWindow; private final Lazy preferencesWindow; private final Lazy quitWindow; - private final Provider unlockWindowBuilderProvider; - private final Provider lockWindowBuilderProvider; + private final Provider unlockWorkflowBuilderProvider; + private final Provider lockWorkflowBuilderProvider; private final Optional trayIntegration; private final Optional appearanceProvider; private final VaultService vaultService; @@ -55,12 +55,12 @@ public class FxApplication extends Application { private final UiAppearanceListener systemInterfaceThemeListener = this::systemInterfaceThemeChanged; @Inject - FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, Provider unlockWindowBuilderProvider, Provider lockWindowBuilderProvider, Lazy quitWindow, Optional trayIntegration, Optional appearanceProvider, VaultService vaultService, LicenseHolder licenseHolder) { + FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, Provider unlockWorkflowBuilderProvider, Provider lockWorkflowBuilderProvider, Lazy quitWindow, Optional trayIntegration, Optional appearanceProvider, VaultService vaultService, LicenseHolder licenseHolder) { this.settings = settings; this.mainWindow = mainWindow; this.preferencesWindow = preferencesWindow; - this.unlockWindowBuilderProvider = unlockWindowBuilderProvider; - this.lockWindowBuilderProvider = lockWindowBuilderProvider; + this.unlockWorkflowBuilderProvider = unlockWorkflowBuilderProvider; + this.lockWorkflowBuilderProvider = lockWorkflowBuilderProvider; this.quitWindow = quitWindow; this.trayIntegration = trayIntegration; this.appearanceProvider = appearanceProvider; @@ -113,14 +113,14 @@ public class FxApplication extends Application { public void startUnlockWorkflow(Vault vault, Optional owner) { Platform.runLater(() -> { - unlockWindowBuilderProvider.get().vault(vault).owner(owner).build().startUnlockWorkflow(); + unlockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startUnlockWorkflow(); LOG.debug("Showing UnlockWindow for {}", vault.getDisplayName()); }); } public void startLockWorkflow(Vault vault, Optional owner) { Platform.runLater(() -> { - lockWindowBuilderProvider.get().vault(vault).owner(owner).build().startLockWorkflow(); + lockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startLockWorkflow(); LOG.debug("Start lock workflow for {}", vault.getDisplayName()); }); } From 0840695e0a3b4808a6f1f2fe26a78a63ff6a0047 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 8 Apr 2021 17:27:45 +0200 Subject: [PATCH 09/30] Refactor lock/unlock convinience methods in FxApplication: * execute vault state transition here * on failed transition show error window * only start worfklow on successful transition --- .../cryptomator/common/vaults/VaultState.java | 9 +++++++- .../cryptomator/ui/fxapp/FxApplication.java | 22 ++++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java index 8966e6f65..b9c3cc9b5 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java @@ -1,6 +1,8 @@ package org.cryptomator.common.vaults; import com.google.common.base.Preconditions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.inject.Inject; import javafx.application.Platform; @@ -11,6 +13,8 @@ import java.util.concurrent.atomic.AtomicReference; @PerVault public class VaultState extends ObservableValueBase implements ObservableObjectValue { + private static final Logger LOG = LoggerFactory.getLogger(VaultState.class); + public enum Value { /** * No vault found at the provided path @@ -46,7 +50,7 @@ public class VaultState extends ObservableValueBase implements private final AtomicReference value; @Inject - public VaultState(VaultState.Value initialValue){ + public VaultState(VaultState.Value initialValue) { this.value = new AtomicReference<>(initialValue); } @@ -62,6 +66,7 @@ public class VaultState extends ObservableValueBase implements /** * Transitions from fromState to toState. + * * @param fromState Previous state * @param toState New state * @return true if successful @@ -71,6 +76,8 @@ public class VaultState extends ObservableValueBase implements boolean success = value.compareAndSet(fromState, toState); if (success) { fireValueChangedEvent(); + } else { + LOG.debug("Failed transiting into state {}: Expected state was {}, but actual state is {}.", fromState, toState, value.get()); } return success; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java index d33b1171c..b0ea8da47 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java @@ -5,11 +5,13 @@ import org.cryptomator.common.LicenseHolder; import org.cryptomator.common.settings.Settings; import org.cryptomator.common.settings.UiTheme; import org.cryptomator.common.vaults.Vault; +import org.cryptomator.common.vaults.VaultState; import org.cryptomator.integrations.tray.TrayIntegrationProvider; import org.cryptomator.integrations.uiappearance.Theme; import org.cryptomator.integrations.uiappearance.UiAppearanceException; import org.cryptomator.integrations.uiappearance.UiAppearanceListener; import org.cryptomator.integrations.uiappearance.UiAppearanceProvider; +import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.VaultService; import org.cryptomator.ui.lock.LockComponent; import org.cryptomator.ui.mainwindow.MainWindowComponent; @@ -46,6 +48,7 @@ public class FxApplication extends Application { private final Lazy quitWindow; private final Provider unlockWorkflowBuilderProvider; private final Provider lockWorkflowBuilderProvider; + private final ErrorComponent.Builder errorWindowBuilder; private final Optional trayIntegration; private final Optional appearanceProvider; private final VaultService vaultService; @@ -55,13 +58,14 @@ public class FxApplication extends Application { private final UiAppearanceListener systemInterfaceThemeListener = this::systemInterfaceThemeChanged; @Inject - FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, Provider unlockWorkflowBuilderProvider, Provider lockWorkflowBuilderProvider, Lazy quitWindow, Optional trayIntegration, Optional appearanceProvider, VaultService vaultService, LicenseHolder licenseHolder) { + FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, Provider unlockWorkflowBuilderProvider, Provider lockWorkflowBuilderProvider, Lazy quitWindow, ErrorComponent.Builder errorWindowBuilder, Optional trayIntegration, Optional appearanceProvider, VaultService vaultService, LicenseHolder licenseHolder) { this.settings = settings; this.mainWindow = mainWindow; this.preferencesWindow = preferencesWindow; this.unlockWorkflowBuilderProvider = unlockWorkflowBuilderProvider; this.lockWorkflowBuilderProvider = lockWorkflowBuilderProvider; this.quitWindow = quitWindow; + this.errorWindowBuilder = errorWindowBuilder; this.trayIntegration = trayIntegration; this.appearanceProvider = appearanceProvider; this.vaultService = vaultService; @@ -113,15 +117,23 @@ public class FxApplication extends Application { public void startUnlockWorkflow(Vault vault, Optional owner) { Platform.runLater(() -> { - unlockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startUnlockWorkflow(); - LOG.debug("Showing UnlockWindow for {}", vault.getDisplayName()); + if (vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING)) { + unlockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startUnlockWorkflow(); + LOG.debug("Start unlock workflow for {}", vault.getDisplayName()); + } else { + showMainWindow().thenAccept(mainWindow -> errorWindowBuilder.window(mainWindow).cause(new IllegalStateException("Unable to unlock vault in non-locked state."))); + } }); } public void startLockWorkflow(Vault vault, Optional owner) { Platform.runLater(() -> { - lockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startLockWorkflow(); - LOG.debug("Start lock workflow for {}", vault.getDisplayName()); + if (vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING)) { + lockWorkflowBuilderProvider.get().vault(vault).owner(owner).build().startLockWorkflow(); + LOG.debug("Start lock workflow for {}", vault.getDisplayName()); + } else { + showMainWindow().thenAccept(mainWindow -> errorWindowBuilder.window(mainWindow).cause(new IllegalStateException("Unable to lock vault in non-unlocked state."))); + } }); } From 41d2a2c77eac69d522aeb9e5078317d0633b1b94 Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Sat, 10 Apr 2021 23:16:00 +0200 Subject: [PATCH 10/30] Added .idea/uiDesigner.xml to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 900a21ae0..d0e6b59a3 100644 --- a/.gitignore +++ b/.gitignore @@ -21,5 +21,6 @@ pom.xml.versionsBackup .idea/compiler.xml .idea/encodings.xml .idea/jarRepositories.xml +.idea/uiDesigner.xml .idea/**/libraries/ *.iml \ No newline at end of file From b066b4b045a4a62498602c77023659abb4ad37fa Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Mon, 12 Apr 2021 18:06:09 +0200 Subject: [PATCH 11/30] opening a vault in read-only always assumes a filename length limit of 220 references #1605 --- .../org/cryptomator/common/vaults/Vault.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 98a6cf21b..ebdda094a 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -104,19 +104,27 @@ public class Vault { if (vaultSettings.usesReadOnlyMode().get()) { flags.add(FileSystemFlags.READONLY); } - if (!flags.contains(FileSystemFlags.READONLY) && vaultSettings.filenameLengthLimit().get() == -1) { + + int usedFilenameLengthLimit; + var fileSystemCapabilityChecker = new FileSystemCapabilityChecker(); + if (flags.contains(FileSystemFlags.READONLY)) { + usedFilenameLengthLimit = Constants.MAX_CIPHERTEXT_NAME_LENGTH; + } else if (vaultSettings.filenameLengthLimit().get() == -1) { LOG.debug("Determining file name length limitations..."); - int limit = new FileSystemCapabilityChecker().determineSupportedFileNameLength(getPath()); - vaultSettings.filenameLengthLimit().set(limit); - LOG.info("Storing file name length limit of {}", limit); + usedFilenameLengthLimit = fileSystemCapabilityChecker.determineSupportedFileNameLength(getPath()); + vaultSettings.filenameLengthLimit().set(usedFilenameLengthLimit); + LOG.info("Storing file name length limit of {}", usedFilenameLengthLimit); + } else { + usedFilenameLengthLimit = vaultSettings.filenameLengthLimit().get(); } - assert vaultSettings.filenameLengthLimit().get() > 0; + + assert usedFilenameLengthLimit > 0; CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() // .withPassphrase(passphrase) // .withFlags(flags) // .withMasterkeyFilename(MASTERKEY_FILENAME) // .withMaxPathLength(vaultSettings.filenameLengthLimit().get() + Constants.MAX_ADDITIONAL_PATH_LENGTH) // - .withMaxNameLength(vaultSettings.filenameLengthLimit().get()) // + .withMaxNameLength(usedFilenameLengthLimit) // .build(); return CryptoFileSystemProvider.newFileSystem(getPath(), fsProps); } From cd5c55aad72123d34ace4c3b9a669830d5003cf2 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 13 Apr 2021 11:22:42 +0200 Subject: [PATCH 12/30] Refactor lock/unlock workflows: * don't set vault state on successful lock workflow * improved error handling --- .../org/cryptomator/ui/lock/LockWorkflow.java | 23 +++---- .../cryptomator/ui/unlock/UnlockWorkflow.java | 60 ++++++++----------- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index 673b74641..fecbe42c4 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -4,6 +4,7 @@ import dagger.Lazy; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; +import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.UserInteractionLock; @@ -35,14 +36,16 @@ public class LockWorkflow extends Task { private final UserInteractionLock forceLockDecisionLock; private final Lazy lockForcedScene; private final Lazy lockFailedScene; + private final ErrorComponent.Builder errorComponent; @Inject - public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene) { + public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene, ErrorComponent.Builder errorComponent) { this.lockWindow = lockWindow; this.vault = vault; this.forceLockDecisionLock = forceLockDecisionLock; this.lockForcedScene = lockForcedScene; this.lockFailedScene = lockFailedScene; + this.errorComponent = errorComponent; } @Override @@ -77,23 +80,23 @@ public class LockWorkflow extends Task { return forceLockDecisionLock.awaitInteraction(); } - @Override - protected void scheduled() { - vault.stateProperty().transition(VaultState.Value.UNLOCKED, VaultState.Value.PROCESSING); - } - @Override protected void succeeded() { LOG.info("Lock of {} succeeded.", vault.getDisplayName()); - vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); + //DO NOT SET VAULT STATE HERE, this is done by the vault internally } @Override protected void failed() { - LOG.warn("Failed to lock {}.", vault.getDisplayName()); + final var throwable = super.getException(); + LOG.warn("Lock of {} failed.", vault.getDisplayName(), throwable); vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); - lockWindow.setScene(lockFailedScene.get()); - lockWindow.show(); + if (throwable instanceof Volume.VolumeException) { + lockWindow.setScene(lockFailedScene.get()); + lockWindow.show(); + } else { + errorComponent.cause(throwable).window(lockWindow).build().showErrorScene(); + } } @Override diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index 08ae1fd4c..2815ad3c4 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -73,22 +73,15 @@ public class UnlockWorkflow extends Task { this.successScene = successScene; this.invalidMountPointScene = invalidMountPointScene; this.errorComponent = errorComponent; - - setOnFailed(event -> { - Throwable throwable = event.getSource().getException(); - if (throwable instanceof InvalidMountPointException e) { - handleInvalidMountPoint(e); - } else { - handleGenericError(throwable); - } - }); } @Override protected Boolean call() throws InterruptedException, IOException, VolumeException, InvalidMountPointException { try { if (attemptUnlock()) { - handleSuccess(); + if (savePassword.get()) { + savePasswordToSystemkeychain(); //savePassword will be wiped on method return, so it must be set here + } return true; } else { cancel(false); // set Tasks state to cancelled @@ -131,24 +124,6 @@ public class UnlockWorkflow extends Task { return passwordEntryLock.awaitInteraction(); } - private void handleSuccess() { - LOG.info("Unlock of '{}' succeeded.", vault.getDisplayName()); - if (savePassword.get()) { - savePasswordToSystemkeychain(); - } - switch (vault.getVaultSettings().actionAfterUnlock().get()) { - case ASK -> Platform.runLater(() -> { - window.setScene(successScene.get()); - window.show(); - }); - case REVEAL -> { - Platform.runLater(window::close); - vaultService.reveal(vault); - } - case IGNORE -> Platform.runLater(window::close); - } - } - private void savePasswordToSystemkeychain() { if (keychain.isSupported()) { try { @@ -193,7 +168,7 @@ public class UnlockWorkflow extends Task { private void handleGenericError(Throwable e) { LOG.error("Unlock failed for technical reasons.", e); - errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene(); + errorComponent.cause(e).window(window).build().showErrorScene(); } private void wipePassword(char[] pw) { @@ -202,23 +177,40 @@ public class UnlockWorkflow extends Task { } } - @Override - protected void scheduled() { - vault.stateProperty().transition(VaultState.Value.LOCKED, VaultState.Value.PROCESSING); - } - @Override protected void succeeded() { + LOG.info("Unlock of '{}' succeeded.", vault.getDisplayName()); + + switch (vault.getVaultSettings().actionAfterUnlock().get()) { + case ASK -> Platform.runLater(() -> { + window.setScene(successScene.get()); + window.show(); + }); + case REVEAL -> { + Platform.runLater(window::close); + vaultService.reveal(vault); + } + case IGNORE -> Platform.runLater(window::close); + } + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.UNLOCKED); } @Override protected void failed() { + LOG.info("Unlock of '{}' failed.", vault.getDisplayName()); + Throwable throwable = super.getException(); + if (throwable instanceof InvalidMountPointException e) { + handleInvalidMountPoint(e); + } else { + handleGenericError(throwable); + } vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override protected void cancelled() { + LOG.debug("Unlock of '{}' canceled.", vault.getDisplayName()); vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } From 642816b631e8b1a4e2fefbdecb5771a74eb4e92b Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 14 Apr 2021 17:26:04 +0200 Subject: [PATCH 13/30] rebuild tray menu when vaultname changes --- .../java/org/cryptomator/ui/traymenu/TrayMenuController.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java b/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java index 96529c5fb..4ce3808c4 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java @@ -44,6 +44,9 @@ class TrayMenuController { public void initTrayMenu() { vaults.addListener(this::vaultListChanged); + vaults.forEach(v -> { + v.displayNameProperty().addListener(this::vaultListChanged); + }); rebuildMenu(); } From 03886f88e8f1d35e36a8d28e97e04216610b573f Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 15 Apr 2021 10:14:28 +0200 Subject: [PATCH 14/30] Fix lock workflow for webdav: * internally, wait for condition that onExit-Method is exceuted (with timeout) * store and execute onExitAction also for webdav --- .../org/cryptomator/common/vaults/Vault.java | 27 +++++++++++++++++++ .../common/vaults/WebDavVolume.java | 10 ++++--- .../org/cryptomator/ui/lock/LockWorkflow.java | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 39cf6d2fd..72c1c7774 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -42,7 +42,11 @@ import java.util.EnumSet; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -70,6 +74,8 @@ public class Vault { private final StringBinding accessPoint; private final BooleanBinding accessPointPresent; private final BooleanProperty showingStats; + private final Lock lock = new ReentrantLock(); //FIXME: naming + private final Condition isLocked = lock.newCondition(); //FIXME: improve naming private volatile Volume volume; @@ -146,6 +152,13 @@ public class Vault { if (throwable != null) { LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); } + lock.lock(); + try { + isLocked.signal(); + } finally { + lock.unlock(); + } + }); } catch (Exception e) { destroyCryptoFileSystem(); @@ -163,6 +176,20 @@ public class Vault { volume.unmount(); } destroyCryptoFileSystem(); + lock.lock(); + try { + while (state.get() != VaultState.Value.LOCKED) { + if (!isLocked.await(3000, TimeUnit.MILLISECONDS)) { + throw new VolumeException("Locking failed"); //FIXME: other exception + } + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new VolumeException("Lock failed."); //FIXME: other/new exception + } finally { + lock.unlock(); + } + } public void reveal(Volume.Revealer vaultRevealer) throws VolumeException { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java index 352cfb489..03c83377c 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/WebDavVolume.java @@ -17,8 +17,6 @@ import java.net.InetAddress; import java.net.UnknownHostException; import java.nio.file.Path; import java.util.Optional; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionStage; import java.util.function.Consumer; import java.util.function.Supplier; @@ -34,6 +32,7 @@ public class WebDavVolume implements Volume { private WebDavServer server; private WebDavServletController servlet; private Mounter.Mount mount; + private Consumer onExitAction; @Inject public WebDavVolume(Provider serverProvider, VaultSettings vaultSettings, Settings settings, WindowsDriveLetters windowsDriveLetters) { @@ -47,9 +46,10 @@ public class WebDavVolume implements Volume { public void mount(CryptoFileSystem fs, String mountFlags, Consumer onExitAction) throws VolumeException { startServlet(fs); mountServlet(); + this.onExitAction = onExitAction; } - private void startServlet(CryptoFileSystem fs){ + private void startServlet(CryptoFileSystem fs) { if (server == null) { server = serverProvider.get(); } @@ -69,7 +69,7 @@ public class WebDavVolume implements Volume { //on windows, prevent an automatic drive letter selection in the upstream library. Either we choose already a specifc one or there is no free. Supplier driveLetterSupplier; - if(System.getProperty("os.name").toLowerCase().contains("windows") && vaultSettings.winDriveLetter().isEmpty().get()) { + if (System.getProperty("os.name").toLowerCase().contains("windows") && vaultSettings.winDriveLetter().isEmpty().get()) { driveLetterSupplier = () -> windowsDriveLetters.getAvailableDriveLetter().orElse(null); } else { driveLetterSupplier = () -> vaultSettings.winDriveLetter().get(); @@ -104,6 +104,7 @@ public class WebDavVolume implements Volume { throw new VolumeException(e); } cleanup(); + onExitAction.accept(null); } @Override @@ -114,6 +115,7 @@ public class WebDavVolume implements Volume { throw new VolumeException(e); } cleanup(); + onExitAction.accept(null); } @Override diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index fecbe42c4..5566caeb0 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -83,7 +83,7 @@ public class LockWorkflow extends Task { @Override protected void succeeded() { LOG.info("Lock of {} succeeded.", vault.getDisplayName()); - //DO NOT SET VAULT STATE HERE, this is done by the vault internally + vault.stateProperty().transition(VaultState.Value.PROCESSING, VaultState.Value.LOCKED); } @Override From 24baa44e709234c7be2b698ecb9a63c034037be4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 15 Apr 2021 10:30:29 +0200 Subject: [PATCH 15/30] stronger encapsulation of vault state await/signal mechanism --- .../org/cryptomator/common/vaults/Vault.java | 23 ++-------- .../cryptomator/common/vaults/VaultState.java | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index 72c1c7774..f10eb3968 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -44,9 +44,6 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; import static org.cryptomator.common.Constants.MASTERKEY_FILENAME; @@ -74,8 +71,6 @@ public class Vault { private final StringBinding accessPoint; private final BooleanBinding accessPointPresent; private final BooleanProperty showingStats; - private final Lock lock = new ReentrantLock(); //FIXME: naming - private final Condition isLocked = lock.newCondition(); //FIXME: improve naming private volatile Volume volume; @@ -152,13 +147,6 @@ public class Vault { if (throwable != null) { LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); } - lock.lock(); - try { - isLocked.signal(); - } finally { - lock.unlock(); - } - }); } catch (Exception e) { destroyCryptoFileSystem(); @@ -176,20 +164,15 @@ public class Vault { volume.unmount(); } destroyCryptoFileSystem(); - lock.lock(); try { - while (state.get() != VaultState.Value.LOCKED) { - if (!isLocked.await(3000, TimeUnit.MILLISECONDS)) { - throw new VolumeException("Locking failed"); //FIXME: other exception - } + boolean locked = state.awaitState(VaultState.Value.LOCKED, 3000, TimeUnit.MILLISECONDS); + if (!locked) { + throw new VolumeException("Locking failed"); //FIXME: other exception } } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new VolumeException("Lock failed."); //FIXME: other/new exception - } finally { - lock.unlock(); } - } public void reveal(Volume.Revealer vaultRevealer) throws VolumeException { diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java index b9c3cc9b5..c3c616307 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java @@ -8,7 +8,11 @@ import javax.inject.Inject; import javafx.application.Platform; import javafx.beans.value.ObservableObjectValue; import javafx.beans.value.ObservableValueBase; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; @PerVault public class VaultState extends ObservableValueBase implements ObservableObjectValue { @@ -48,6 +52,8 @@ public class VaultState extends ObservableValueBase implements } private final AtomicReference value; + private final Lock lock = new ReentrantLock(); + private final Condition valueChanged = lock.newCondition(); @Inject public VaultState(VaultState.Value initialValue) { @@ -89,7 +95,43 @@ public class VaultState extends ObservableValueBase implements } } + /** + * Waits for the specified time, until the desired state is reached. + * + * @param desiredState what state to wait for + * @param time the maximum time to wait + * @param unit the time unit of the {@code time} argument + * @return {@code false} if the waiting time detectably elapsed before reaching {@code desiredState} + * @throws InterruptedException if the current thread is interrupted + */ + public boolean awaitState(Value desiredState, long time, TimeUnit unit) throws InterruptedException { + lock.lock(); + try { + long remaining = unit.convert(time, TimeUnit.NANOSECONDS); + while (value.get() != desiredState) { + if (remaining <= 0L) { + return false; + } + remaining = valueChanged.awaitNanos(remaining); + } + return true; + } finally { + lock.unlock(); + } + } + + private void signal() { + lock.lock(); + try { + valueChanged.signalAll(); + } finally { + lock.unlock(); + } + } + + @Override protected void fireValueChangedEvent() { + signal(); if (Platform.isFxApplicationThread()) { super.fireValueChangedEvent(); } else { From 22a0d3a9a5d5c955a1720c120cfe86168e2c54b4 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 15 Apr 2021 11:11:05 +0200 Subject: [PATCH 16/30] bump fuse/dokany-nio versions --- main/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main/pom.xml b/main/pom.xml index 51ed64413..9f71aaeab 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -30,8 +30,8 @@ 1.0.0-beta2 1.0.0-beta2 1.0.0-beta1 - 1.4.0-SNAPSHOT - 1.3.0-SNAPSHOT + 1.3.1 + 1.3.0 1.2.0 From 0d00520ac1baa2d6f62c4c2ef16a58ddc5e67e70 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Thu, 15 Apr 2021 12:54:19 +0200 Subject: [PATCH 17/30] Throw specifc exception on not completed lock of a vault --- .../vaults/LockNotCompletedException.java | 12 ++++++++ .../org/cryptomator/common/vaults/Vault.java | 28 +++++++++++-------- .../cryptomator/ui/common/VaultService.java | 3 +- .../ui/launcher/AppLifecycleListener.java | 3 ++ .../org/cryptomator/ui/lock/LockWorkflow.java | 5 ++-- 5 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/vaults/LockNotCompletedException.java diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/LockNotCompletedException.java b/main/commons/src/main/java/org/cryptomator/common/vaults/LockNotCompletedException.java new file mode 100644 index 000000000..237630da0 --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/LockNotCompletedException.java @@ -0,0 +1,12 @@ +package org.cryptomator.common.vaults; + +public class LockNotCompletedException extends Exception { + + public LockNotCompletedException(String reason) { + super(reason); + } + + public LockNotCompletedException(Throwable cause) { + super(cause); + } +} diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java index f10eb3968..fae92b3b7 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/Vault.java @@ -140,14 +140,7 @@ public class Vault { cryptoFileSystem.set(fs); try { volume = volumeProvider.get(); - volume.mount(fs, getEffectiveMountFlags(), throwable -> { - LOG.info("Unmounted vault '{}'", getDisplayName()); - destroyCryptoFileSystem(); - state.set(VaultState.Value.LOCKED); - if (throwable != null) { - LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), throwable); - } - }); + volume.mount(fs, getEffectiveMountFlags(), this::lockOnVolumeExit); } catch (Exception e) { destroyCryptoFileSystem(); throw e; @@ -157,21 +150,32 @@ public class Vault { } } - public synchronized void lock(boolean forced) throws VolumeException { + private void lockOnVolumeExit(Throwable t) { + LOG.info("Unmounted vault '{}'", getDisplayName()); + destroyCryptoFileSystem(); + state.set(VaultState.Value.LOCKED); + if (t != null) { + LOG.warn("Unexpected unmount and lock of vault " + getDisplayName(), t); + } + } + + public synchronized void lock(boolean forced) throws VolumeException, LockNotCompletedException { + //initiate unmount if (forced && volume.supportsForcedUnmount()) { volume.unmountForced(); } else { volume.unmount(); } - destroyCryptoFileSystem(); + + //wait for lockOnVolumeExit to be executed try { boolean locked = state.awaitState(VaultState.Value.LOCKED, 3000, TimeUnit.MILLISECONDS); if (!locked) { - throw new VolumeException("Locking failed"); //FIXME: other exception + throw new LockNotCompletedException("Locking of vault " + this.getDisplayName() + " still in progress."); } } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new VolumeException("Lock failed."); //FIXME: other/new exception + throw new LockNotCompletedException(e); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java index 6555904e5..b81ddec49 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.common; +import org.cryptomator.common.vaults.LockNotCompletedException; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; @@ -175,7 +176,7 @@ public class VaultService { } @Override - protected Vault call() throws Volume.VolumeException { + protected Vault call() throws Volume.VolumeException, LockNotCompletedException { vault.lock(forced); return vault; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java index 5e2ed383a..75af409f1 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/AppLifecycleListener.java @@ -1,6 +1,7 @@ package org.cryptomator.ui.launcher; import org.cryptomator.common.ShutdownHook; +import org.cryptomator.common.vaults.LockNotCompletedException; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; @@ -129,6 +130,8 @@ public class AppLifecycleListener { vault.lock(true); } catch (Volume.VolumeException e) { LOG.error("Failed to unmount vault " + vault.getPath(), e); + } catch (LockNotCompletedException e) { + LOG.error("Failed to lock vault " + vault.getPath(), e); } } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index 5566caeb0..87fd486f2 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -1,6 +1,7 @@ package org.cryptomator.ui.lock; import dagger.Lazy; +import org.cryptomator.common.vaults.LockNotCompletedException; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; @@ -49,10 +50,10 @@ public class LockWorkflow extends Task { } @Override - protected Void call() throws Volume.VolumeException, InterruptedException { + protected Void call() throws Volume.VolumeException, InterruptedException, LockNotCompletedException { try { vault.lock(false); - } catch (Volume.VolumeException e) { + } catch (Volume.VolumeException | LockNotCompletedException e) { LOG.debug("Regular lock of {} failed.", vault.getDisplayName(), e); var decision = askUserForAction(); switch (decision) { From 673fdcd095f7770e1a66313c11f5d6a5f6c52163 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 15 Apr 2021 13:45:20 +0200 Subject: [PATCH 18/30] stupid. --- .../src/main/java/org/cryptomator/common/vaults/VaultState.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java index c3c616307..d83a3ae8e 100644 --- a/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java +++ b/main/commons/src/main/java/org/cryptomator/common/vaults/VaultState.java @@ -107,7 +107,7 @@ public class VaultState extends ObservableValueBase implements public boolean awaitState(Value desiredState, long time, TimeUnit unit) throws InterruptedException { lock.lock(); try { - long remaining = unit.convert(time, TimeUnit.NANOSECONDS); + long remaining = TimeUnit.NANOSECONDS.convert(time, unit); while (value.get() != desiredState) { if (remaining <= 0L) { return false; From 3376b16b7bc86022d69e23561221ccabb9aada7f Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Fri, 16 Apr 2021 12:18:03 +0200 Subject: [PATCH 19/30] Rename Donation Key to Supporter Certificate (#1613) Renamed all occurences of donation key to supporter certificate and adjust ui. Co-authored-by: Tobias Hagemann Co-authored-by: Sebastian Stenzel --- .../mainwindow/MainWindowTitleController.java | 2 +- .../GeneralPreferencesController.java | 4 ++-- .../ui/preferences/PreferencesController.java | 4 ++-- .../ui/preferences/PreferencesModule.java | 4 ++-- .../ui/preferences/SelectedPreferencesTab.java | 4 ++-- ...ava => SupporterCertificateController.java} | 18 +++++++++--------- .../src/main/resources/fxml/preferences.fxml | 4 ++-- ...ionkey.fxml => preferences_contribute.fxml} | 10 +++++----- .../resources/fxml/preferences_general.fxml | 2 +- .../src/main/resources/i18n/strings.properties | 13 ++++++++----- 10 files changed, 34 insertions(+), 31 deletions(-) rename main/ui/src/main/java/org/cryptomator/ui/preferences/{DonationKeyPreferencesController.java => SupporterCertificateController.java} (68%) rename main/ui/src/main/resources/fxml/{preferences_donationkey.fxml => preferences_contribute.fxml} (72%) diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowTitleController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowTitleController.java index 5a5bb59a6..ea9311c88 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowTitleController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowTitleController.java @@ -95,7 +95,7 @@ public class MainWindowTitleController implements FxController { @FXML public void showDonationKeyPreferences() { - application.showPreferencesWindow(SelectedPreferencesTab.DONATION_KEY); + application.showPreferencesWindow(SelectedPreferencesTab.CONTRIBUTE); } /* Getter/Setter */ diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java index ea1fbfe3a..64d71a8b7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java @@ -157,8 +157,8 @@ public class GeneralPreferencesController implements FxController { @FXML - public void showDonationTab() { - selectedTabProperty.set(SelectedPreferencesTab.DONATION_KEY); + public void showContributeTab() { + selectedTabProperty.set(SelectedPreferencesTab.CONTRIBUTE); } @FXML diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesController.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesController.java index 64d5c991b..276794753 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesController.java @@ -26,7 +26,7 @@ public class PreferencesController implements FxController { public Tab generalTab; public Tab volumeTab; public Tab updatesTab; - public Tab donationKeyTab; + public Tab contributeTab; public Tab aboutTab; @Inject @@ -52,7 +52,7 @@ public class PreferencesController implements FxController { return switch (selectedTab) { case UPDATES -> updatesTab; case VOLUME -> volumeTab; - case DONATION_KEY -> donationKeyTab; + case CONTRIBUTE -> contributeTab; case GENERAL -> generalTab; case ABOUT -> aboutTab; case ANY -> updateAvailable.get() ? updatesTab : generalTab; diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java index 51fa6b581..5fee567b7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java @@ -77,8 +77,8 @@ abstract class PreferencesModule { @Binds @IntoMap - @FxControllerKey(DonationKeyPreferencesController.class) - abstract FxController bindDonationKeyPreferencesController(DonationKeyPreferencesController controller); + @FxControllerKey(SupporterCertificateController.class) + abstract FxController bindSupporterCertificatePreferencesController(SupporterCertificateController controller); @Binds @IntoMap diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/SelectedPreferencesTab.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/SelectedPreferencesTab.java index a76f2ac1c..892d16a8c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/SelectedPreferencesTab.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/SelectedPreferencesTab.java @@ -22,9 +22,9 @@ public enum SelectedPreferencesTab { UPDATES, /** - * Show donation key tab + * Show contribute tab */ - DONATION_KEY, + CONTRIBUTE, /** * Show about tab diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/DonationKeyPreferencesController.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/SupporterCertificateController.java similarity index 68% rename from main/ui/src/main/java/org/cryptomator/ui/preferences/DonationKeyPreferencesController.java rename to main/ui/src/main/java/org/cryptomator/ui/preferences/SupporterCertificateController.java index a4814ec82..02b8bab91 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/DonationKeyPreferencesController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/SupporterCertificateController.java @@ -14,17 +14,17 @@ import javafx.scene.control.TextArea; import javafx.scene.control.TextFormatter; @PreferencesScoped -public class DonationKeyPreferencesController implements FxController { +public class SupporterCertificateController implements FxController { - private static final String DONATION_URI = "https://store.cryptomator.org/desktop"; + private static final String SUPPORTER_URI = "https://store.cryptomator.org/desktop"; private final Application application; private final LicenseHolder licenseHolder; private final Settings settings; - public TextArea donationKeyField; + public TextArea supporterCertificateField; @Inject - DonationKeyPreferencesController(Application application, LicenseHolder licenseHolder, Settings settings) { + SupporterCertificateController(Application application, LicenseHolder licenseHolder, Settings settings) { this.application = application; this.licenseHolder = licenseHolder; this.settings = settings; @@ -32,9 +32,9 @@ public class DonationKeyPreferencesController implements FxController { @FXML public void initialize() { - donationKeyField.setText(licenseHolder.getLicenseKey().orElse(null)); - donationKeyField.textProperty().addListener(this::registrationKeyChanged); - donationKeyField.setTextFormatter(new TextFormatter<>(this::checkVaultNameLength)); + supporterCertificateField.setText(licenseHolder.getLicenseKey().orElse(null)); + supporterCertificateField.textProperty().addListener(this::registrationKeyChanged); + supporterCertificateField.setTextFormatter(new TextFormatter<>(this::checkVaultNameLength)); } private TextFormatter.Change checkVaultNameLength(TextFormatter.Change change) { @@ -53,8 +53,8 @@ public class DonationKeyPreferencesController implements FxController { } @FXML - public void getDonationKey() { - application.getHostServices().showDocument(DONATION_URI); + public void getSupporterCertificate() { + application.getHostServices().showDocument(SUPPORTER_URI); } public LicenseHolder getLicenseHolder() { diff --git a/main/ui/src/main/resources/fxml/preferences.fxml b/main/ui/src/main/resources/fxml/preferences.fxml index c0cd2b6c7..643b31176 100644 --- a/main/ui/src/main/resources/fxml/preferences.fxml +++ b/main/ui/src/main/resources/fxml/preferences.fxml @@ -38,12 +38,12 @@ - + - + diff --git a/main/ui/src/main/resources/fxml/preferences_donationkey.fxml b/main/ui/src/main/resources/fxml/preferences_contribute.fxml similarity index 72% rename from main/ui/src/main/resources/fxml/preferences_donationkey.fxml rename to main/ui/src/main/resources/fxml/preferences_contribute.fxml index 087d9da26..bfe91d0eb 100644 --- a/main/ui/src/main/resources/fxml/preferences_donationkey.fxml +++ b/main/ui/src/main/resources/fxml/preferences_contribute.fxml @@ -12,7 +12,7 @@ @@ -24,7 +24,7 @@ - + @@ -33,8 +33,8 @@ - -