Sfoglia il codice sorgente

Graceful unmounting on Windows and improved error handling of deferred closables.

Sebastian Stenzel 8 anni fa
parent
commit
4c3c60060d

+ 4 - 1
main/frontend-api/src/main/java/org/cryptomator/frontend/Frontend.java

@@ -24,7 +24,10 @@ public interface Frontend extends AutoCloseable {
 
 	void mount(Map<MountParam, Optional<String>> map) throws CommandFailedException;
 
-	void unmount() throws CommandFailedException;
+	/**
+	 * Unmounts the file system and stops any file system handler threads.
+	 */
+	void close() throws Exception;
 
 	void reveal() throws CommandFailedException;
 

+ 2 - 2
main/frontend-webdav/src/main/java/org/cryptomator/frontend/webdav/WebDavFrontend.java

@@ -55,10 +55,10 @@ class WebDavFrontend implements Frontend {
 		mount = webdavMounterProvider.chooseMounter(mountParams).mount(uri, mountParams);
 	}
 
-	@Override
-	public void unmount() throws CommandFailedException {
+	private void unmount() throws CommandFailedException {
 		if (mount != null) {
 			mount.unmount();
+			mount = null;
 		}
 	}
 

+ 1 - 1
main/frontend-webdav/src/main/java/org/cryptomator/frontend/webdav/mount/WindowsWebDavMounter.java

@@ -160,7 +160,7 @@ final class WindowsWebDavMounter implements WebDavMounterStrategy {
 		private WindowsWebDavMount(String driveLetter) {
 			this.driveLetter = CharUtils.toCharacterObject(driveLetter);
 			this.openExplorerScript = fromLines("start explorer.exe " + driveLetter + ":");
-			this.unmountScript = fromLines("net use " + driveLetter + ": /delete");
+			this.unmountScript = fromLines("net use " + driveLetter + ": /delete /no");
 		}
 
 		@Override

+ 10 - 1
main/ui/src/main/java/org/cryptomator/ui/CryptomatorModule.java

@@ -23,6 +23,8 @@ import org.cryptomator.ui.model.VaultObjectMapperProvider;
 import org.cryptomator.ui.settings.Settings;
 import org.cryptomator.ui.settings.SettingsProvider;
 import org.cryptomator.ui.util.DeferredCloser;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 
@@ -34,6 +36,7 @@ import javafx.stage.Stage;
 @Module(includes = {CryptoEngineModule.class, CommonsModule.class, WebDavModule.class})
 class CryptomatorModule {
 
+	private static final Logger LOG = LoggerFactory.getLogger(CryptomatorModule.class);
 	private final Application application;
 	private final Stage mainWindow;
 
@@ -59,7 +62,13 @@ class CryptomatorModule {
 	@Singleton
 	DeferredCloser provideDeferredCloser() {
 		DeferredCloser closer = new DeferredCloser();
-		Cryptomator.addShutdownTask(closer::close);
+		Cryptomator.addShutdownTask(() -> {
+			try {
+				closer.close();
+			} catch (Exception e) {
+				LOG.error("Error during shutdown.", e);
+			}
+		});
 		return closer;
 	}
 

+ 6 - 1
main/ui/src/main/java/org/cryptomator/ui/MainApplication.java

@@ -12,6 +12,7 @@ import java.io.IOException;
 import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.concurrent.ExecutionException;
 
 import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.ui.controllers.MainController;
@@ -126,7 +127,11 @@ public class MainApplication extends Application {
 
 	@Override
 	public void stop() {
-		closer.close();
+		try {
+			closer.close();
+		} catch (ExecutionException e) {
+			LOG.error("Error closing ressources", e);
+		}
 	}
 
 }

+ 3 - 4
main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockedController.java

@@ -117,12 +117,11 @@ public class UnlockedController extends LocalizedFXMLViewController {
 	@FXML
 	private void didClickLockVault(ActionEvent event) {
 		asyncTaskService.asyncTaskOf(() -> {
-			vault.get().unmount();
 			vault.get().deactivateFrontend();
-		}).onError(CommandFailedException.class, () -> {
-			messageLabel.setText(localization.getString("unlocked.label.unmountFailed"));
-		}).andFinally(() -> {
+		}).onSuccess(() -> {
 			listener.ifPresent(listener -> listener.didLock(this));
+		}).onError(Exception.class, () -> {
+			messageLabel.setText(localization.getString("unlocked.label.unmountFailed"));
 		}).run();
 	}
 

+ 1 - 5
main/ui/src/main/java/org/cryptomator/ui/model/Vault.java

@@ -148,7 +148,7 @@ public class Vault implements CryptoFileSystemDelegate {
 		}
 	}
 
-	public synchronized void deactivateFrontend() {
+	public synchronized void deactivateFrontend() throws Exception {
 		filesystemFrontend.close();
 		statsFileSystem = Optional.empty();
 		Platform.runLater(() -> unlocked.set(false));
@@ -168,10 +168,6 @@ public class Vault implements CryptoFileSystemDelegate {
 		Optionals.ifPresent(filesystemFrontend.get(), Frontend::reveal);
 	}
 
-	public void unmount() throws CommandFailedException {
-		Optionals.ifPresent(filesystemFrontend.get(), Frontend::unmount);
-	}
-
 	// ******************************************************************************
 	// Delegate methods
 	// ********************************************************************************/

+ 0 - 6
main/ui/src/main/java/org/cryptomator/ui/util/DeferredClosable.java

@@ -28,12 +28,6 @@ public interface DeferredClosable<T> extends AutoCloseable {
 	 */
 	public Optional<T> get();
 
-	/**
-	 * Quietly closes the Object. If the object was closed before, nothing
-	 * happens.
-	 */
-	public void close();
-
 	/**
 	 * @return an empty object.
 	 */

+ 31 - 24
main/ui/src/main/java/org/cryptomator/ui/util/DeferredCloser.java

@@ -13,12 +13,10 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentSkipListMap;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.cryptomator.common.ConsumerThrowingException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
 
@@ -31,7 +29,7 @@ import com.google.common.annotations.VisibleForTesting;
  * 
  * <p>
  * If you have a {@link DeferredCloser} instance present, call
- * {@link #closeLater(Object, Closer)} immediately after you have opened the
+ * {@link #closeLater(Object, ConsumerThrowingException)} immediately after you have opened the
  * resource and return a resource handle. If {@link #close()} is called, the
  * resource will be closed. Calling {@link DeferredClosable#close()} on the resource
  * handle will also close the resource and prevent a second closing by
@@ -42,8 +40,6 @@ import com.google.common.annotations.VisibleForTesting;
  */
 public class DeferredCloser implements AutoCloseable {
 
-	private static final Logger LOG = LoggerFactory.getLogger(DeferredCloser.class);
-
 	@VisibleForTesting
 	final Map<Long, ManagedResource<?>> cleanups = new ConcurrentSkipListMap<>();
 
@@ -51,33 +47,32 @@ public class DeferredCloser implements AutoCloseable {
 	final AtomicLong counter = new AtomicLong();
 
 	private class ManagedResource<T> implements DeferredClosable<T> {
+		
 		private final long number = counter.incrementAndGet();
-
-		private final AtomicReference<T> object = new AtomicReference<>();
+		private final T object;
 		private final ConsumerThrowingException<T, Exception> closer;
+		private boolean closed = false;
 
 		public ManagedResource(T object, ConsumerThrowingException<T, Exception> closer) {
 			super();
-			this.object.set(object);
-			this.closer = closer;
+			this.object = Objects.requireNonNull(object);
+			this.closer = Objects.requireNonNull(closer);
 		}
 
 		@Override
-		public void close() {
-			final T oldObject = object.getAndSet(null);
-			if (oldObject != null) {
-				cleanups.remove(number);
-				try {
-					closer.accept(oldObject);
-				} catch (Exception e) {
-					LOG.error("Closing resource failed.", e);
-				}
-			}
+		public synchronized void close() throws Exception {
+			closer.accept(object);
+			cleanups.remove(number);
+			closed = true;
 		}
 
 		@Override
 		public Optional<T> get() throws IllegalStateException {
-			return Optional.ofNullable(object.get());
+			if (closed) {
+				return Optional.empty();
+			} else {
+				return Optional.of(object);
+			}
 		}
 	}
 
@@ -85,11 +80,23 @@ public class DeferredCloser implements AutoCloseable {
 	 * Closes all added objects which have not been closed before and releases references.
 	 */
 	@Override
-	public void close() {
+	public void close() throws ExecutionException {
+		ExecutionException exception = null;
 		for (Iterator<ManagedResource<?>> iterator = cleanups.values().iterator(); iterator.hasNext();) {
 			final ManagedResource<?> closableProvider = iterator.next();
-			closableProvider.close();
-			iterator.remove();
+			try {
+				closableProvider.close();
+				iterator.remove();
+			} catch (Exception e) {
+				if (exception == null) {
+					exception = new ExecutionException(e);
+				} else {
+					exception.addSuppressed(e);
+				}
+			}
+		}
+		if (exception != null) {
+			throw exception;
 		}
 	}