From 497de6e85067df1ad05297a5a55bc05f1d7094a9 Mon Sep 17 00:00:00 2001 From: Charlie Groves Date: Thu, 24 Jan 2008 00:59:24 +0000 Subject: [PATCH] Switch to using a file lock on gettingdown.lock in the app's dir to keep multiple instances of getdown from running at the same time instead of monitoring getdown.txt's modtime. The modtime is still used to see if another getdown ran while this one waited. --- .../threerings/getdown/data/Application.java | 107 +++++++++--------- .../threerings/getdown/launcher/Getdown.java | 34 ++++-- .../getdown/tools/JarDiffPatcher.java | 2 +- 3 files changed, 81 insertions(+), 62 deletions(-) diff --git a/src/java/com/threerings/getdown/data/Application.java b/src/java/com/threerings/getdown/data/Application.java index 7bd3c44..3ed4983 100644 --- a/src/java/com/threerings/getdown/data/Application.java +++ b/src/java/com/threerings/getdown/data/Application.java @@ -22,15 +22,6 @@ package com.threerings.getdown.data; import java.awt.Color; import java.awt.Rectangle; - -import javax.swing.JApplet; - -import java.lang.reflect.Method; -import java.security.AllPermission; -import java.security.CodeSource; -import java.security.PermissionCollection; -import java.security.Permissions; - import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; @@ -40,14 +31,20 @@ import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; - +import java.io.RandomAccessFile; +import java.lang.reflect.Method; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.security.AllPermission; +import java.security.CodeSource; import java.security.GeneralSecurityException; +import java.security.PermissionCollection; +import java.security.Permissions; import java.security.Signature; import java.security.cert.Certificate; - import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -56,16 +53,16 @@ import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; -import com.samskivert.io.StreamUtil; -import com.samskivert.jdbc.depot.clause.UpdateClause; -import com.samskivert.text.MessageUtil; -import com.samskivert.util.ArrayIntSet; -import com.samskivert.util.RunAnywhere; -import com.samskivert.util.StringUtil; +import javax.swing.JApplet; import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.IOUtils; +import com.samskivert.io.StreamUtil; +import com.samskivert.text.MessageUtil; +import com.samskivert.util.ArrayIntSet; +import com.samskivert.util.RunAnywhere; +import com.samskivert.util.StringUtil; import com.threerings.getdown.Log; import com.threerings.getdown.launcher.MultipleGetdownRunning; import com.threerings.getdown.launcher.RotatingBackgrounds; @@ -1027,49 +1024,53 @@ public class Application protected void downloadConfigFile () throws IOException { - requireNoOtherGetdownRunning(); downloadControlFile(CONFIG_FILE, false); - updateConfigModtime(); } /** - * Checks the modtime on CONFIG_FILE and returns whether it has changed since the last time - * this method was called. + * @return true if gettingdown.lock was locked or was already locked by this application. */ - public boolean checkForAnotherGetdown () + public synchronized boolean lockForUpdates () { - File config = getLocalPath(CONFIG_FILE); - if (_lastConfigModtime != -1 && _lastConfigModtime < config.lastModified()) { + if (_lock != null && _lock.isValid()) { return true; } - _lastConfigModtime = config.lastModified(); - return false; - } - - /** - * Calls {@link #checkForAnotherGetdown} and throws a {@link MultipleGetdownRunning} if it - * detects that another Getdown instance is running. - */ - public void requireNoOtherGetdownRunning () - throws MultipleGetdownRunning - { - if (checkForAnotherGetdown()) { - throw new MultipleGetdownRunning(); + try { + _lockChannel = new RandomAccessFile(getLocalPath("gettingdown.lock"), "rw").getChannel(); + } catch (FileNotFoundException e) { + Log.warning("Unable to create lock file[message=" + e.getMessage() + "]"); + Log.logStackTrace(e); + return false; } + try { + _lock = _lockChannel.tryLock(); + } catch (IOException e) { + Log.warning("Unable to create lock[message=" + e.getMessage() + "]"); + Log.logStackTrace(e); + } + return _lock != null; } /** - * Updates the modtime on CONFIG_FILE to now and notes that this application saw it at that - * time for use in checkForAnotherGetdown + * Release gettingdown.lock */ - public void updateConfigModtime () + public synchronized void releaseLock () { - File config = getLocalPath(CONFIG_FILE); - _lastConfigModtime = System.currentTimeMillis(); - if (!config.setLastModified(_lastConfigModtime) && !_warnedAboutSetLastModified) { - Log.warning("Unable to set modtime on config file, will be unable to check for " + - "other instances of getdown running."); - _warnedAboutSetLastModified = true; + if (_lock != null) { + try { + _lock.release(); + } catch (IOException e) { + Log.warning("Unable to release lock[message=" + e.getMessage() + "]"); + Log.logStackTrace(e); + } + try { + _lockChannel.close(); + } catch (IOException e) { + Log.warning("Unable to close lock channel[message=" + e.getMessage() + "]"); + Log.logStackTrace(e); + } + _lockChannel = null; + _lock = null; } } @@ -1162,11 +1163,6 @@ public class Application } } - // Check that another getdown hasn't started running since we started downloading this - // file. The rename will obliterate the modtime we're tracking to keep multiple instances - // from running. - requireNoOtherGetdownRunning(); - // now move the temporary file over the original File original = getLocalPath(path); if (!FileUtil.renameTo(target, original)) { @@ -1309,8 +1305,11 @@ public class Application /** If a warning has been issued about not being able to set modtimes. */ protected boolean _warnedAboutSetLastModified; - /** The modtime on CONFIG_FILE last time it was checked, or -1 if it hasn't been checked. */ - protected long _lastConfigModtime = -1; - protected static final String[] SA_PROTO = new String[0]; + + /** Locks gettingdown.lock in the app dir. Held the entire time updating is going on.*/ + protected FileLock _lock; + + /** Channel to the file underying _lock. Kept around solely so the lock doesn't close. */ + protected FileChannel _lockChannel; } diff --git a/src/java/com/threerings/getdown/launcher/Getdown.java b/src/java/com/threerings/getdown/launcher/Getdown.java index 2b5e58c..ac969e8 100644 --- a/src/java/com/threerings/getdown/launcher/Getdown.java +++ b/src/java/com/threerings/getdown/launcher/Getdown.java @@ -362,17 +362,32 @@ public abstract class Getdown extends Thread // now force our UI to be recreated with the updated info createInterface(true); } - _app.requireNoOtherGetdownRunning(); - // Update the modtime here to stake a claim that we're going to getdown eventually - _app.updateConfigModtime(); + if (!_app.lockForUpdates()) { + throw new MultipleGetdownRunning(); + } + + // Update the config modtime so a sleeping getdown will notice the change. + File config = _app.getLocalPath(Application.CONFIG_FILE); + if (!config.setLastModified(System.currentTimeMillis())) { + Log.warning("Unable to set modtime on config file, will be unable to check for " + + "another instance of getdown running while this one waits."); + } if (_delay > 0) { + // don't hold the lock while waiting, let another getdown proceed if it starts. + _app.releaseLock(); + // Store the config modtime before waiting the delay amount of time + long lastConfigModtime = config.lastModified(); try { Log.info("Waiting " + _delay + " minutes before beginning actual work"); - Thread.sleep(_delay * 60 * 1000); + Thread.sleep(_delay * 15 * 1000); } catch (InterruptedException ie) { Log.warning("Who dares disturb my slumber?"); Log.logStackTrace(ie); } + if (lastConfigModtime < config.lastModified()) { + Log.warning("getdown.txt was modified while getdown was waiting"); + throw new MultipleGetdownRunning(); + } } // we create this tracking counter here so that we properly note the first time through @@ -416,7 +431,9 @@ public abstract class Getdown extends Thread // Only launch if we aren't in silent mode. Some mystery program starting out // of the blue would be disconcerting. if (!_silent || _launchInSilent) { - _app.requireNoOtherGetdownRunning(); + // One last check for the lock before launching. It'll already be held + // unless we're in silent mode. + _app.lockForUpdates(); launch(); } return; @@ -634,8 +651,11 @@ public abstract class Getdown extends Thread public boolean downloadProgress (int percent, long remaining) { // check for another getdown running at 0 and every 10% after that if (_lastCheck == -1 || percent >= _lastCheck + 10) { - if (_app.checkForAnotherGetdown()) { - return false; + if (_delay > 0){ + // Stop the presses if something else is holding the lock. + boolean locked = _app.lockForUpdates(); + _app.releaseLock(); + return locked; } _lastCheck = percent; } diff --git a/src/java/com/threerings/getdown/tools/JarDiffPatcher.java b/src/java/com/threerings/getdown/tools/JarDiffPatcher.java index 5bb55c4..d712d51 100644 --- a/src/java/com/threerings/getdown/tools/JarDiffPatcher.java +++ b/src/java/com/threerings/getdown/tools/JarDiffPatcher.java @@ -139,7 +139,7 @@ public class JarDiffPatcher for (int j = 0; j < keys.length; j++) { // Apply move command String newName = (String)keys[j]; - String oldName = (String)renameMap.get(newName); + String oldName = renameMap.get(newName); // Get source JarEntry JarEntry oldEntry = oldJar.getJarEntry(oldName);