Close mmap for ElevationProvider if not removed (#1809)

* elevation: if temp files should be kept, still release memory mapped files * minor fix to remove them if specified
graphhopper · Nov 28, 2019 · 80af196 · 80af196
1 parent 7bd3e76
commit 80af196
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 42 deletions.
diff --git a/core/src/main/java/com/graphhopper/reader/dem/AbstractSRTMElevationProvider.java b/core/src/main/java/com/graphhopper/reader/dem/AbstractSRTMElevationProvider.java
@@ -60,18 +60,13 @@ int calcIntKey(double lat, double lon) {
     @Override
     public void release() {
         cacheData.clear();
-
-        // for memory mapped type we create temporary unpacked files which should be removed
-        if (autoRemoveTemporary && dir != null)
-            dir.clear();
-    }
-
-    /**
-     * Creating temporary files can take a long time to fill our DataAccess object, so this option
-     * can be used to disable the default clear mechanism via specifying 'false'.
-     */
-    public void setAutoRemoveTemporaryFiles(boolean autoRemoveTemporary) {
-        this.autoRemoveTemporary = autoRemoveTemporary;
+        if (dir != null) {
+            // for memory mapped type we remove temporary files
+            if (autoRemoveTemporary)
+                dir.clear();
+            else
+                dir.close();
+        }
     }
 
     int down(double val) {

diff --git a/core/src/main/java/com/graphhopper/reader/dem/AbstractTiffElevationProvider.java b/core/src/main/java/com/graphhopper/reader/dem/AbstractTiffElevationProvider.java
@@ -57,10 +57,13 @@ public AbstractTiffElevationProvider(String baseUrl, String cacheDir, String dow
     @Override
     public void release() {
         cacheData.clear();
-
-        // for memory mapped type we create temporary unpacked files which should be removed
-        if (autoRemoveTemporary && dir != null)
-            dir.clear();
+        if (dir != null) {
+            // for memory mapped type we remove temporary files
+            if (autoRemoveTemporary)
+                dir.clear();
+            else
+                dir.close();
+        }
     }
 
     /**
@@ -186,14 +189,4 @@ private void fillDataAccessWithElevationData(Raster raster, DataAccess heights,
             throw new RuntimeException("Problem at x:" + x + ", y:" + y, ex);
         }
     }
-
-    /**
-     * Creating temporary files can take a long time as we need to unpack tiff as well as to fill
-     * our DataAccess object, so this option can be used to disable the default clear mechanism via
-     * specifying 'false'.
-     */
-    public void setAutoRemoveTemporaryFiles(boolean autoRemoveTemporary) {
-        this.autoRemoveTemporary = autoRemoveTemporary;
-    }
-
 }
diff --git a/core/src/main/java/com/graphhopper/storage/Directory.java b/core/src/main/java/com/graphhopper/storage/Directory.java
@@ -47,11 +47,6 @@ public interface Directory {
 
     DataAccess find(String name, DAType type);
 
-    /**
-     * Renames the specified DataAccess object into one.
-     */
-    // DataAccess rename( DataAccess da, String newName );
-
     /**
      * Removes the specified object from the directory.
      */
@@ -67,6 +62,11 @@ public interface Directory {
      */
     void clear();
 
+    /**
+     * Releases all allocated resources from the directory without removing backing files.
+     */
+    void close();
+
     /**
      * Returns all created directories.
      */

diff --git a/core/src/main/java/com/graphhopper/storage/GHDirectory.java b/core/src/main/java/com/graphhopper/storage/GHDirectory.java
@@ -106,30 +106,36 @@ public DataAccess find(String name, DAType type) {
         return da;
     }
 
+    @Override
+    public void close() {
+        for (DataAccess da : map.values()) {
+            da.close();
+        }
+        map.clear();
+    }
+
     @Override
     public void clear() {
         for (DataAccess da : map.values()) {
-            removeDA(da, da.getName());
+            da.close();
+            removeBackingFile(da, da.getName());
         }
         map.clear();
     }
 
     @Override
     public void remove(DataAccess da) {
-        removeFromMap(da.getName());
-        removeDA(da, da.getName());
-    }
+        DataAccess old = map.remove(da.getName());
+        if (old == null)
+            throw new IllegalStateException("Couldn't remove DataAccess: " + da.getName());
 
-    void removeDA(DataAccess da, String name) {
         da.close();
-        if (da.getType().isStoring())
-            removeDir(new File(location + name));
+        removeBackingFile(da, da.getName());
     }
 
-    void removeFromMap(String name) {
-        DataAccess da = map.remove(name);
-        if (da == null)
-            throw new IllegalStateException("Couldn't remove dataAccess object:" + name);
+    private void removeBackingFile(DataAccess da, String name) {
+        if (da.getType().isStoring())
+            removeDir(new File(location + name));
     }
 
     @Override