Merge pull request #249 from mikeprimm/master

Tighten up logic for assuring unlock of locked files during exceptions, fix version number display

src/main/java/org/dynmap/DynmapWorld.java

@@ -301,8 +301,9 @@ public class DynmapWorld {
                     im = ImageIO.read(f);
                 } catch (IOException e) {
                 } catch (IndexOutOfBoundsException e) {
-                }
+                } finally {
                     FileLockManager.releaseReadLock(f);
+                }
                 if(im != null) {
                     im.getRGB(0, 0, width, height, argb, 0, width);    /* Read data */
                     im.flush();
@@ -333,6 +334,7 @@ public class DynmapWorld {
             }
         }
         FileLockManager.getWriteLock(zf);
+        try {
             TileHashManager hashman = MapManager.mapman.hashman;
             long crc = hashman.calculateTileHash(kzIm.argb_buf); /* Get hash of tile */
             int tilex = ztx/step/2;
@@ -353,7 +355,9 @@ public class DynmapWorld {
                 MapManager.mapman.pushUpdate(this.world, new Client.Tile(zfname));
                 enqueueZoomOutUpdate(zf, pd.zoomlevel+1);
             }
+        } finally {
             FileLockManager.releaseWriteLock(zf);
             KzedMap.freeBufferedImage(kzIm);
         }
     }
+}

src/main/java/org/dynmap/MapManager.java

@@ -137,7 +137,7 @@ public class MapManager {
         MapTile tile = null;
         int rendercnt = 0;
         CommandSender sender;
-        long starttime;
+        long timeaccum;
 
         /* Full world, all maps render */
         FullWorldRenderState(DynmapWorld dworld, Location l, CommandSender sender) {
@@ -173,7 +173,7 @@ public class MapManager {
                 /* If render queue is empty, start next map */
                 if(renderQueue.isEmpty()) {
                     if(map_index >= 0) { /* Finished a map? */
-                        double msecpertile = (double)(tstart - starttime) / (double)((rendercnt>0)?rendercnt:1);
+                        double msecpertile = (double)timeaccum / (double)((rendercnt>0)?rendercnt:1);
                         sender.sendMessage("Full render of map '" + world.maps.get(map_index).getClass().getSimpleName() + "' of world '" +
                                  world.world.getName() + "' completed - " + rendercnt + " tiles rendered (" + String.format("%.2f", msecpertile) + " msec/tile).");
                     }                	
@@ -187,7 +187,6 @@ public class MapManager {
                         return;
                     }
                     map = world.maps.get(map_index);
-                    starttime = System.currentTimeMillis();
 
                     /* Now, prime the render queue */
                     for (MapTile mt : map.getTiles(loc)) {
@@ -241,8 +240,9 @@ public class MapManager {
                 found.remove(tile);
                 if(!cache.isEmpty()) {
                     rendercnt++;
+                    timeaccum += System.currentTimeMillis() - tstart;
                     if((rendercnt % 100) == 0) {
-                        double msecpertile = (double)(System.currentTimeMillis() - starttime) / (double)rendercnt;
+                        double msecpertile = (double)timeaccum / (double)rendercnt;
                         sender.sendMessage("Full render of map '" + world.maps.get(map_index).getClass().getSimpleName() + "' on world '" +
                             w.getName() + "' in progress - " + rendercnt + " tiles rendered (" + String.format("%.2f", msecpertile) + " msec/tile).");
                     }

src/main/java/org/dynmap/flat/FlatMap.java

@@ -271,10 +271,11 @@ public class FlatMap extends MapType {
             }
         }
         /* Test to see if we're unchanged from older tile */
-        FileLockManager.getWriteLock(outputFile);
         TileHashManager hashman = MapManager.mapman.hashman;
         long crc = hashman.calculateTileHash(argb_buf);
         boolean tile_update = false;
+        FileLockManager.getWriteLock(outputFile);
+        try {
             if((!outputFile.exists()) || (crc != hashman.getImageHashCode(tile.getKey(), null, t.x, t.y))) {
                 /* Wrap buffer as buffered image */
                 Debug.debug("saving image " + outputFile.getPath());
@@ -295,15 +296,18 @@ public class FlatMap extends MapType {
             else {
                 Debug.debug("skipping image " + outputFile.getPath() + " - hash match");
             }
-        KzedMap.freeBufferedImage(im);
+        } finally {
             FileLockManager.releaseWriteLock(outputFile);
+            KzedMap.freeBufferedImage(im);
+        }
         MapManager.mapman.updateStatistics(tile, null, true, tile_update, !rendered);
 
         /* If day too, handle it */
         if(night_and_day) {
             File dayfile = new File(tile.getDynmapWorld().worldtilepath, tile.getDayFilename());
-            FileLockManager.getWriteLock(dayfile);
             crc = hashman.calculateTileHash(argb_buf_day);
+            FileLockManager.getWriteLock(dayfile);
+            try {
                 if((!dayfile.exists()) || (crc != hashman.getImageHashCode(tile.getKey(), "day", t.x, t.y))) {
                     Debug.debug("saving image " + dayfile.getPath());
                     if(!dayfile.getParentFile().exists())
@@ -324,8 +328,10 @@ public class FlatMap extends MapType {
                     Debug.debug("skipping image " + dayfile.getPath() + " - hash match");
                     tile_update = false;
                 }
-            KzedMap.freeBufferedImage(im_day);
+            } finally {
                 FileLockManager.releaseWriteLock(dayfile);
+                KzedMap.freeBufferedImage(im_day);
+            }
             MapManager.mapman.updateStatistics(tile, "day", true, tile_update, !rendered);
         }
         

src/main/java/org/dynmap/kzedmap/DefaultTileRenderer.java

@@ -256,12 +256,13 @@ public class DefaultTileRenderer implements MapTileRenderer {
         int oy = (mtile.py == zmtile.getTileY())?0:KzedMap.tileHeight/2;
 
         /* Test to see if we're unchanged from older tile */
-        FileLockManager.getWriteLock(fname);
         TileHashManager hashman = MapManager.mapman.hashman;
         long crc = hashman.calculateTileHash(img.argb_buf);
         boolean updated_fname = false;
         int tx = mtile.px/KzedMap.tileWidth;
         int ty = mtile.py/KzedMap.tileHeight;
+        FileLockManager.getWriteLock(fname);
+        try {
             if((!fname.exists()) || (crc != hashman.getImageHashCode(mtile.getKey(), null, tx, ty))) {
                 Debug.debug("saving image " + fname.getPath());
                 if(!fname.getParentFile().exists())
@@ -277,8 +278,10 @@ public class DefaultTileRenderer implements MapTileRenderer {
                 hashman.updateHashCode(mtile.getKey(), null, tx, ty, crc);
                 updated_fname = true;
             }
-        KzedMap.freeBufferedImage(img);
+        } finally {
             FileLockManager.releaseWriteLock(fname);
+            KzedMap.freeBufferedImage(img);
+        }
         MapManager.mapman.updateStatistics(mtile, null, true, updated_fname, !rendered);
 
         mtile.file = fname;
@@ -287,8 +290,9 @@ public class DefaultTileRenderer implements MapTileRenderer {
         
         File dfname = new File(mtile.getDynmapWorld().worldtilepath, mtile.getDayFilename());
         if(img_day != null) {
-            FileLockManager.getWriteLock(dfname);
             crc = hashman.calculateTileHash(img.argb_buf);
+            FileLockManager.getWriteLock(dfname);
+            try {
                 if((!dfname.exists()) || (crc != hashman.getImageHashCode(mtile.getKey(), "day", tx, ty))) {
                     Debug.debug("saving image " + dfname.getPath());
                     if(!dfname.getParentFile().exists())
@@ -304,8 +308,10 @@ public class DefaultTileRenderer implements MapTileRenderer {
                     hashman.updateHashCode(mtile.getKey(), "day", tx, ty, crc);
                     updated_dfname = true;
                 }
-            KzedMap.freeBufferedImage(img_day);
+            } finally {
                 FileLockManager.releaseWriteLock(dfname);
+                KzedMap.freeBufferedImage(img_day);
+            }
             MapManager.mapman.updateStatistics(mtile, "day", true, updated_dfname, !rendered);
         }
         
@@ -313,6 +319,7 @@ public class DefaultTileRenderer implements MapTileRenderer {
         // make the zoomed tile here
         boolean ztile_updated = false;
         FileLockManager.getWriteLock(zoomFile);
+        try {
             if(updated_fname || (!zoomFile.exists())) {
                 saveZoomedTile(zmtile, zoomFile, zimg, ox, oy, null);
                 MapManager.mapman.pushUpdate(zmtile.getWorld(),
@@ -320,14 +327,17 @@ public class DefaultTileRenderer implements MapTileRenderer {
                 zmtile.getDynmapWorld().enqueueZoomOutUpdate(zoomFile);
                 ztile_updated = true;
             }
-        KzedMap.freeBufferedImage(zimg);
+        } finally {
             FileLockManager.releaseWriteLock(zoomFile);
+            KzedMap.freeBufferedImage(zimg);
+        }
         MapManager.mapman.updateStatistics(zmtile, null, true, ztile_updated, !rendered);
         
         if(zimg_day != null) {
             File zoomFile_day = new File(zmtile.getDynmapWorld().worldtilepath, zmtile.getDayFilename());
             ztile_updated = false;
             FileLockManager.getWriteLock(zoomFile_day);
+            try {
                 if(updated_dfname || (!zoomFile_day.exists())) {
                     saveZoomedTile(zmtile, zoomFile_day, zimg_day, ox, oy, "day");
                     MapManager.mapman.pushUpdate(zmtile.getWorld(),
@@ -335,8 +345,10 @@ public class DefaultTileRenderer implements MapTileRenderer {
                     zmtile.getDynmapWorld().enqueueZoomOutUpdate(zoomFile_day);
                     ztile_updated = true;
                 }
-            KzedMap.freeBufferedImage(zimg_day);
+            } finally {
                 FileLockManager.releaseWriteLock(zoomFile_day);
+                KzedMap.freeBufferedImage(zimg_day);
+            }
             MapManager.mapman.updateStatistics(zmtile, "day", true, ztile_updated, !rendered);
         }
     }

src/main/java/org/dynmap/utils/FileLockManager.java

@@ -18,7 +18,7 @@ public class FileLockManager {
      * Get write lock on file - exclusive lock, no other writers or readers
      * @throws InterruptedException
      */
-    public static void getWriteLock(File f) {
+    public static boolean getWriteLock(File f) {
         String fn = f.getPath();
         synchronized(lock) {
             boolean got_lock = false;
@@ -29,6 +29,7 @@ public class FileLockManager {
                         lock.wait(); 
                     } catch (InterruptedException ix) {
                         Log.severe("getWriteLock(" + fn + ") interrupted");
+                        return false;
                     }
                 }
                 else {
@@ -38,6 +39,7 @@ public class FileLockManager {
             }
         }
         //Log.info("getWriteLock(" + f + ")");
+        return true;
     }
     /**
      * Release write lock
@@ -60,10 +62,17 @@ public class FileLockManager {
     /**
      * Get read lock on file - multiple readers allowed, blocks writers
      */
-    public static void getReadLock(File f) {
+    public static boolean getReadLock(File f) {
+        return getReadLock(f, -1);
+    }
+    /**
+     * Get read lock on file - multiple readers allowed, blocks writers - with timeout (msec)
+     */
+    public static boolean getReadLock(File f, long timeout) {
         String fn = f.getPath();
         synchronized(lock) {
             boolean got_lock = false;
+            boolean first_wait = true;
             while(!got_lock) {
                 Integer lockcnt = filelocks.get(fn);    /* Get lock count */
                 if(lockcnt == null) {
@@ -76,14 +85,23 @@ public class FileLockManager {
                 }
                 else {  /* Write lock in place */
                     try {
+                        if((timeout > 0) && (!first_wait)) {    /* We already waited */
+                            return false;
+                        }
+                        if(timeout < 0)
                             lock.wait();
+                        else
+                            lock.wait(timeout);
+                        first_wait = false;
                     } catch (InterruptedException ix) {
                         Log.severe("getReadLock(" + fn + ") interrupted");
+                        return false;
                     }
                 }
             }
         }        
         //Log.info("getReadLock(" + f + ")");
+        return true;
     }
     /**
      * Release read lock

src/main/java/org/dynmap/web/HttpServerConnection.java

@@ -131,7 +131,6 @@ public class HttpServerConnection extends Thread {
                 HttpRequest request = new HttpRequest();
                 request.rmtaddr = rmtaddr;
                 if (!readRequestHeader(in, request)) {
-                    socket.close();
                     return;
                 }
 
@@ -174,7 +173,6 @@ public class HttpServerConnection extends Thread {
                 }
 
                 if (handler == null) {
-                    socket.close();
                     return;
                 }
 
@@ -186,10 +184,7 @@ public class HttpServerConnection extends Thread {
                     throw e;
                 } catch (Exception e) {
                     Log.severe("HttpHandler '" + handler + "' has thown an exception", e);
-                    if (socket != null) {
                     out.flush();
-                        socket.close();
-                    }
                     return;
                 }
 
@@ -211,37 +206,28 @@ public class HttpServerConnection extends Thread {
                     if (responseBody == null) {
                         Debug.debug("Response was given without Content-Length by '" + handler + "' for path '" + request.path + "'.");
                         out.flush();
-                        socket.close();
                         return;
                     }
                 }
 
+                out.flush();
+
                 if (!isKeepalive) {
-                    out.flush();
-                    socket.close();
                     return;
                 }
-
-                out.flush();
             }
         } catch (IOException e) {
-            if (socket != null) {
+
-                try {
-                    socket.close();
-                } catch (IOException ex) {
-                }
-            }
-            return;
         } catch (Exception e) {
-            if (socket != null) {
-                try {
-                    socket.close();
-                } catch (IOException ex) {
-                }
-            }
             Log.severe("Exception while handling request: ", e);
             e.printStackTrace();
-            return;
+        } finally {
+            if (socket != null) {
+                try {
+                    socket.close();
+                } catch (IOException ex) {
+                }
+            }
         }
     }
 }

src/main/java/org/dynmap/web/handlers/FileHandler.java

@@ -111,20 +111,13 @@ public abstract class FileHandler implements HttpHandler {
                 while ((readBytes = fileInput.read(readBuffer)) > 0) {
                     out.write(readBuffer, 0, readBytes);
                 }
-            } catch (IOException e) {
-                throw e;
             } finally {
                 freeReadBuffer(readBuffer);
-                if(fileInput != null) {
-                    closeFileInput(path, fileInput);
-                    fileInput = null;
             }
-            }
+        } finally {
-        } catch (Exception e) {
             if (fileInput != null) {
                 try { closeFileInput(path, fileInput); fileInput = null; } catch (IOException ex) { }
             }
-            throw e;
         }
     }
 }

src/main/java/org/dynmap/web/handlers/FilesystemHandler.java

@@ -23,14 +23,16 @@ public class FilesystemHandler extends FileHandler {
     @Override
     protected InputStream getFileInput(String path, HttpRequest request, HttpResponse response) {
         File file = new File(root, path);
-        FileLockManager.getReadLock(file);
+        if(!FileLockManager.getReadLock(file, 5000)) {    /* Wait up to 5 seconds for lock */
+            Log.severe("Timeout waiting for lock on file " + file.getPath());
+            return null;
+        }
+        FileInputStream result = null;
         try {
             if (file.getCanonicalPath().startsWith(root.getAbsolutePath()) && file.isFile()) {
-                FileInputStream result;
                 try {
                     result = new FileInputStream(file);
                 } catch (FileNotFoundException e) {
-                    FileLockManager.releaseReadLock(file);
                     return null;
                 }
                 response.fields.put(HttpField.ContentLength, Long.toString(file.length()));
@@ -38,14 +40,17 @@ public class FilesystemHandler extends FileHandler {
             }
         } catch(IOException ex) {
             Log.severe("Unable to get canoical path of requested file.", ex);
+        } finally {
+            if(result == null) FileLockManager.releaseReadLock(file);
         }
-        FileLockManager.releaseReadLock(file);
         return null;
     }
     protected void closeFileInput(String path, InputStream in) throws IOException {
+        try {
             super.closeFileInput(path, in);
+        } finally {
             File file = new File(root, path);
             FileLockManager.releaseReadLock(file);
         }
-
+    }
 }

src/main/resources/plugin.yml

@@ -1,6 +1,6 @@
 name: dynmap
 main: org.dynmap.DynmapPlugin
-version: 0.20
+version: "0.20"
 authors: [FrozenCow, mikeprimm, zeeZ]
 softdepend: [Permissions]
 commands: