sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: Close the data stores when ask to close them and when the application stop.
Date Fri, 01 Nov 2019 13:56:05 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 89f5d43  Close the data stores when ask to close them and when the application stop.
89f5d43 is described below

commit 89f5d43a64d8155d24a37fa63be6d2e283eda5ae
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Fri Nov 1 13:50:16 2019 +0100

    Close the data stores when ask to close them and when the application stop.
---
 .../main/java/org/apache/sis/gui/DataViewer.java   |  2 +-
 .../org/apache/sis/gui/dataset/ResourceTree.java   | 28 ++++---
 .../apache/sis/internal/gui/BackgroundThreads.java | 21 +++++-
 .../apache/sis/internal/gui/ExceptionReporter.java |  2 +-
 .../apache/sis/internal/gui/ResourceLoader.java    | 85 +++++++++++++++++++++-
 .../org/apache/sis/internal/gui/Resources.java     | 12 ++-
 .../apache/sis/internal/gui/Resources.properties   |  4 +-
 .../sis/internal/gui/Resources_fr.properties       |  4 +-
 .../org/apache/sis/internal/system/Modules.java    |  7 +-
 .../java/org/apache/sis/util/collection/Cache.java |  2 +-
 10 files changed, 146 insertions(+), 21 deletions(-)

diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/DataViewer.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/DataViewer.java
index b39b0c8..1f27107 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/DataViewer.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/DataViewer.java
@@ -215,7 +215,7 @@ public class DataViewer extends Application {
      * Invoked when the application should stop. No SIS application can be used after
      * this method has been invoked (i.e. the application can not be restarted).
      *
-     * @throws Exception if an error occurred.
+     * @throws Exception if an error occurred, for example while closing a data store.
      */
     @Override
     public void stop() throws Exception {
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
index 6a43a71..2b62235 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceTree.java
@@ -146,8 +146,8 @@ public class ResourceTree extends TreeView<Resource> {
                     addTo = (Root) root;
                 } else {
                     final TreeItem<Resource> group = new TreeItem<>();
-                    addTo = new Root(group);
-                    addTo.add(root);
+                    addTo = new Root(group, root);
+                    group.setValue(addTo);
                     setRoot(group);
                     setShowRoot(false);
                 }
@@ -236,15 +236,19 @@ public class ResourceTree extends TreeView<Resource> {
     }
 
     /**
-     * Removes the given resource. A resource can be removed only if it is a root.
-     * If the given resource is not in this tree view or is not a root resource,
-     * then this method does nothing.
+     * Removes the given resource from the tree and closes it if it is a {@link DataStore}.
+     * It is caller's responsibility to ensure that the given resource is not used anymore.
+     * A resource can be removed only if it is a root. If the given resource is not in this
+     * tree view or is not a root resource, then this method does nothing.
      *
      * @param  resource  the resource to remove, or {@code null}.
-     * @return whether the resource has been removed.
      */
-    public boolean removeResource(final Resource resource) {
-        return findOrRemove(resource, true);
+    public void removeAndClose(final Resource resource) {
+        if (findOrRemove(resource, true)) {
+            if (resource instanceof DataStore) {
+                ResourceLoader.removeAndClose((DataStore) resource);
+            }
+        }
     }
 
     /**
@@ -463,7 +467,7 @@ public class ResourceTree extends TreeView<Resource> {
          * Invoked when user selected the "close" action in the contextual menu.
          */
         private void close(final ActionEvent event) {
-            ((ResourceTree) getTreeView()).removeResource(getItem());
+            ((ResourceTree) getTreeView()).removeAndClose(getItem());
         }
     }
 
@@ -608,9 +612,13 @@ public class ResourceTree extends TreeView<Resource> {
         /**
          * Creates a new aggregate which is going to be wrapped in the given item.
          * Caller should invoke {@code group.setValue(root)} after this constructor.
+         *
+         * @param  group     the new tree root which will contain "real" resources.
+         * @param  previous  the previous root, to be added in the new group.
          */
-        Root(final TreeItem<Resource> group) {
+        Root(final TreeItem<Resource> group, final Resource previous) {
             components = group.getChildren();
+            add(previous);
         }
 
         /**
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
index 6aa50d8..ebae74d 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
@@ -20,8 +20,11 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.sis.internal.system.Modules;
 import org.apache.sis.internal.system.Threads;
+import org.apache.sis.util.logging.Logging;
 
 
 /**
@@ -70,10 +73,22 @@ public final class BackgroundThreads extends AtomicInteger implements
ThreadFact
 
     /**
      * Invoked at application shutdown time for stopping the executor threads after they
completed their task.
-     * This method returns immediately but the background threads may continue for some time
if they did not
-     * finished their task yet.
+     * This method returns soon but the background threads may continue for some time if
they did not finished
+     * their task yet.
+     *
+     * @throws Exception if an error occurred while closing at least one data store.
      */
-    public static void stop() {
+    public static void stop() throws Exception {
         EXECUTOR.shutdown();
+        try {
+            EXECUTOR.awaitTermination(30, TimeUnit.SECONDS);
+        } catch (InterruptedException e) {
+            /*
+             * Someone does not want to wait for termination.
+             * Closes the data stores now even if some of them may still be in use.
+             */
+            Logging.recoverableException(Logging.getLogger(Modules.APPLICATION), BackgroundThreads.class,
"stop", e);
+        }
+        ResourceLoader.closeAll();
     }
 }
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ExceptionReporter.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ExceptionReporter.java
index 9a3f83b..4030014 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ExceptionReporter.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ExceptionReporter.java
@@ -81,7 +81,7 @@ public final class ExceptionReporter {
      * @param arguments   the arguments for creating the text identified by the {@code text}
key.
      * @param exception   the exception to report.
      */
-    private static void show(final short title, final short text, final Object[] arguments,
final Throwable exception) {
+    static void show(final short title, final short text, final Object[] arguments, final
Throwable exception) {
         if (exception != null) {
             String message = exception.getLocalizedMessage();
             if (message == null) {
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
index 966c577..581dd42 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/ResourceLoader.java
@@ -24,8 +24,11 @@ import java.nio.file.Paths;
 import java.nio.file.Files;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
 import javafx.concurrent.Task;
 import javafx.event.EventHandler;
+import javafx.application.Platform;
 import org.apache.sis.storage.Resource;
 import org.apache.sis.storage.StorageConnector;
 import org.apache.sis.storage.DataStoreException;
@@ -55,7 +58,6 @@ import org.apache.sis.storage.DataStore;
  * and the associated resource is still in memory, it will be returned directly.
  *
  * @todo Set title. Add progress listener and cancellation capability.
- * @todo Need a mechanism for deciding when to close the data store. May need an usage count.
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
@@ -177,4 +179,85 @@ public final class ResourceLoader extends Task<Resource> {
         }
         return name;
     }
+
+    /**
+     * Removes the given data store from cache and closes it. It is caller's responsibility
+     * to ensure that the given data store is not used anymore before to invoke this method.
+     * This method should be invoked from JavaFX thread for making sure there is no new usage
+     * of the given data store starting while we are closing it. However after the data store
+     * has been removed from the cache, the close action is performed in a background thread.
+     *
+     * @param  toClose  the data store to remove from the cache and to close.
+     * @return {@code true} if the value has been removed from the cache, or {@code false}
+     *         if it has not been found. Note that the data store is closed in all cases.
+     */
+    public static boolean removeAndClose(final DataStore toClose) {
+        /*
+         * A simpler code would be as below, but can not be used at this time because our
+         * Cache.entrySet() implementation does not support the Iterator.remove() operation.
+         *
+         * CACHE.values().removeIf((v) -> v == toClose);
+         */
+        boolean removed = false;
+        for (final Cache.Entry<Object,DataStore> entries : CACHE.entrySet()) {
+            if (entries.getValue() == toClose) {
+                removed |= CACHE.remove(entries.getKey(), toClose);
+            }
+        }
+        BackgroundThreads.execute(() -> {
+            try {
+                toClose.close();
+            } catch (final Throwable e) {
+                Platform.runLater(() -> {
+                    ExceptionReporter.show(Resources.Keys.ErrorClosingFile, Resources.Keys.CanNotClose_1,
+                                           new Object[] {toClose.getDisplayName()}, e);
+                });
+            }
+        });
+        return removed;
+    }
+
+    /**
+     * Closes all data stores. This method should be invoked only after all background threads
+     * terminated in case some of them were using a data store. The data stores will be closed
+     * in parallel.
+     *
+     * @throws Exception if an error occurred while closing at least one data store.
+     */
+    static void closeAll() throws Exception {
+        final Closer closer = new Closer();
+        do {
+            Stream.of(CACHE.keySet().toArray()).parallel().forEach(closer);
+        } while (!CACHE.isEmpty());
+        closer.rethrow();
+    }
+
+    /**
+     * The handler in charge of closing the data store and record the failures if some errors
happen.
+     * The same handler instance may be used concurrently while closing many data stores
in parallel.
+     */
+    private static final class Closer implements Consumer<Object> {
+        /** The error that occurred while closing a data store. */
+        private Exception error;
+
+        /** Closes the given data store. */
+        @Override public void accept(final Object source) {
+            final DataStore toClose = CACHE.remove(source);
+            if (source != null) try {
+                toClose.close();
+            } catch (Exception e) {
+                synchronized (this) {
+                    if (error == null) error = e;
+                    else error.addSuppressed(e);
+                }
+            }
+        }
+
+        /** If an error occurred, re-throws that error. */
+        synchronized void rethrow() throws Exception {
+            if (error != null) {
+                throw error;
+            }
+        }
+    }
 }
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.java
index 7ba6995..cd9e617 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.java
@@ -66,7 +66,12 @@ public final class Resources extends IndexedResourceBundle {
         public static final short CRSs = 1;
 
         /**
-         * Can not open “{0}”
+         * Can not close “{0}”. Data may be lost.
+         */
+        public static final short CanNotClose_1 = 12;
+
+        /**
+         * Can not open “{0}”.
          */
         public static final short CanNotReadFile_1 = 5;
 
@@ -76,6 +81,11 @@ public final class Resources extends IndexedResourceBundle {
         public static final short Close = 8;
 
         /**
+         * Error closing file
+         */
+        public static final short ErrorClosingFile = 13;
+
+        /**
          * Error opening file
          */
         public static final short ErrorOpeningFile = 6;
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.properties
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.properties
index 8b7e8b2..8181ae1 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.properties
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources.properties
@@ -21,9 +21,11 @@
 #
 
 AllFiles          = All files
-CanNotReadFile_1  = Can not open \u201c{0}\u201d
+CanNotReadFile_1  = Can not open \u201c{0}\u201d.
+CanNotClose_1     = Can not close \u201c{0}\u201d. Data may be lost.
 Close             = Close
 CRSs              = Coordinate Reference Systems
+ErrorClosingFile  = Error closing file
 ErrorOpeningFile  = Error opening file
 Exit              = Exit
 File              = File
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources_fr.properties
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources_fr.properties
index c1a70fe..097bd4b 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources_fr.properties
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/Resources_fr.properties
@@ -26,9 +26,11 @@
 #
 
 AllFiles          = Tous les fichiers
-CanNotReadFile_1  = Ne peut pas ouvrir \u00ab\u202f{0}\u202f\u00bb
+CanNotReadFile_1  = Ne peut pas ouvrir \u00ab\u202f{0}\u202f\u00bb.
+CanNotClose_1     = Ne peut pas fermer \u00ab\u202f{0}\u202f\u00bb. Il pourrait y avoir une
perte de donn\u00e9es.
 Close             = Fermer
 CRSs              = Syst\u00e8mes de r\u00e9f\u00e9rence des coordonn\u00e9es
+ErrorClosingFile  = Erreur \u00e0 la fermeture du fichier
 ErrorOpeningFile  = Erreur \u00e0 l\u2019ouverture du fichier
 Exit              = Quitter
 File              = Fichier
diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/system/Modules.java b/core/sis-utility/src/main/java/org/apache/sis/internal/system/Modules.java
index bf5d60a..22e59d7 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/internal/system/Modules.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/internal/system/Modules.java
@@ -32,7 +32,7 @@ package org.apache.sis.internal.system;
  * Each constant should be the name of the main package of its corresponding module.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
  * @since   0.3
  * @module
  */
@@ -98,6 +98,11 @@ public final class Modules {
     public static final String CONSOLE = "org.apache.sis.console";
 
     /**
+     * The {@value} module name.
+     */
+    public static final String APPLICATION = "org.apache.sis.gui";
+
+    /**
      * The major version number of all Apache SIS modules.
      *
      * @see org.apache.sis.util.Version
diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java b/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
index a2b464e..fc9e340 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
@@ -1279,7 +1279,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements
ConcurrentMap<K,V> {
     /**
      * Returns the set of entries in this cache. The returned set is subjects to the same
caution
      * than the ones documented in the {@link ConcurrentHashMap#entrySet()} method, except
that
-     * it doesn't support removal of elements (including through the {@link Iterator#remove}
+     * it does not support removal of elements (including through the {@link Iterator#remove}
      * method call).
      *
      * @return a view of the entries contained in this map.


Mime
View raw message