From 1cdbadd0d2ad5509d970bed2fc388327ec2c4521 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Tue, 29 Nov 2011 08:15:11 +0100 Subject: Fix TempJarCache's Multi-User Bug (Reported by Martin Hegedus) It turns out that Java's File mkdir() only makes the directory writable for the current user, I have missed this fact. Great catch. 1. Fix TempJarCache.isInitialized(): Return false if not successfully initialized. It merely returned if it has passed 'initSingleton()' and ignored the staticInitError. 2. Fix TempFileCache pattern of determining the temp base directory We cannot just use a static directory name, due to the multi user environment and user write permissions on File.mkdir(). IOUtil has a new 'getTempDir(..)' methods, which iterates through integers [000000-999999] until a writeable directory could be found or created. TempFileCache initializes this temp base dir in the static block ensuring the value is final for the JVM / ClassLoader. Updated comments/docs in TempFileCache. --- src/java/com/jogamp/common/util/IOUtil.java | 53 ++++ .../jogamp/common/util/cache/TempFileCache.java | 288 +++++++++++---------- .../com/jogamp/common/util/cache/TempJarCache.java | 4 +- 3 files changed, 211 insertions(+), 134 deletions(-) (limited to 'src/java/com/jogamp/common') diff --git a/src/java/com/jogamp/common/util/IOUtil.java b/src/java/com/jogamp/common/util/IOUtil.java index 1f0b2ed..1e81c8e 100644 --- a/src/java/com/jogamp/common/util/IOUtil.java +++ b/src/java/com/jogamp/common/util/IOUtil.java @@ -532,4 +532,57 @@ public class IOUtil { } return tmpRoot; } + + /** + * This methods finds [and creates] a temporary directory: + * 
+     *    for(tempBaseDir = tempRootDir + tmpDirPrefix + _ + [000000-999999]) {
+     *      if(tempBaseDir.isDirectory()) {
+     *          if(tempBaseDir.canWrite()) {
+     *              return tempBaseDir;
+     *          }
+     *      } else {
+     *          tempBaseDir.mkdir();
+     *          return tempBaseDir;
+     *      }
+     *    }
+     *
+ * The tempRootDir is retrieved by {@link #getTempRoot()}. + *

+ * The iteration through [000000-999999] ensures that the code is multi-user save. + *

+ * @param tmpDirPrefix + * @return a temporary directory, writable by this user + * @throws IOException + * @throws SecurityException + */ + public static File getTempDir(String tmpDirPrefix) + throws IOException, SecurityException + { + final File tempRoot = IOUtil.getTempRoot(); + + for(int i = 0; i<=999999; i++) { + final String tmpDirSuffix = String.format("_%06d", i); // 6 digits for iteration + final File tmpBaseDir = new File(tempRoot, tmpDirPrefix+tmpDirSuffix); + if (tmpBaseDir.isDirectory()) { + // existing directory + if(tmpBaseDir.canWrite()) { + // can write - OK + return tmpBaseDir; + } + // not writable, hence used by another user - continue + } else { + // non existing directory, create and validate it + tmpBaseDir.mkdir(); + if (!tmpBaseDir.isDirectory()) { + throw new IOException("Cannot create temp base directory " + tmpBaseDir); + } + if(!tmpBaseDir.canWrite()) { + throw new IOException("Cannot write to created temp base directory " + tmpBaseDir); + } + return tmpBaseDir; // created and writable - OK + } + } + throw new IOException("Could not create temp directory @ "+tempRoot.getAbsolutePath()+tmpDirPrefix+"_*"); + } } diff --git a/src/java/com/jogamp/common/util/cache/TempFileCache.java b/src/java/com/jogamp/common/util/cache/TempFileCache.java index 9c2556d..4210a24 100644 --- a/src/java/com/jogamp/common/util/cache/TempFileCache.java +++ b/src/java/com/jogamp/common/util/cache/TempFileCache.java @@ -41,26 +41,27 @@ import jogamp.common.Debug; public class TempFileCache { private static final boolean DEBUG = Debug.debug("TempFileCache"); - // Lifecycle: For all JVMs, ClassLoader and times. + // Flag indicating that we got a fatal error in the static initializer. + private static boolean staticInitError = false; + private static final String tmpDirPrefix = "jogamp.tmp.cache"; + // Lifecycle: For one user's JVMs, ClassLoader and time. + private static final File tmpBaseDir; + // Get the value of the tmproot system property - // Lifecycle: For all JVMs and ClassLoader + // Lifecycle: For one user's concurrently running JVMs and ClassLoader /* package */ static final String tmpRootPropName = "jnlp.jogamp.tmp.cache.root"; - // Flag indicating that we got a fatal error in the static initializer. - private static boolean staticInitError = false; - - private static File tmpBaseDir; - // String representing the name of the temp root directory relative to the // tmpBaseDir. Its value is "jlnNNNNN", which is the unique filename created // by File.createTempFile() without the ".tmp" extension. // - // Lifecycle: For all JVMs and ClassLoader + // Lifecycle: For one user's concurrently running JVMs and ClassLoader // private static String tmpRootPropValue; + // Lifecycle: For one user's concurrently running JVMs and ClassLoader private static File tmpRootDir; // Flag indicating that we got a fatal error in the initializer. @@ -69,15 +70,39 @@ public class TempFileCache { private File individualTmpDir; static { - // Create / initialize the temp root directory, starting the Reaper - // thread to reclaim old installations if necessary. If we get an - // exception, set an error code. - try { - initTmpRoot(); - } catch (Exception ex) { - ex.printStackTrace(); - staticInitError = true; - } + // Global Lock ! + synchronized (System.out) { + // Create / initialize the temp root directory, starting the Reaper + // thread to reclaim old installations if necessary. If we get an + // exception, set an error code. + File _tmpBaseDir = null; + try { + _tmpBaseDir = IOUtil.getTempDir(tmpDirPrefix); // Retrieve the tmpbase directory. + } catch (Exception ex) { + System.err.println("Warning: Catched Exception while retrieving temp base directory:"); + ex.printStackTrace(); + staticInitError = true; + } + tmpBaseDir = _tmpBaseDir; + + if (DEBUG) { + System.err.println("TempFileCache: Static Initialization ---------------------------------------------- OK: "+(!staticInitError)); + System.err.println("TempFileCache: Thread: "+Thread.currentThread().getName()+", CL 0x"+Integer.toHexString(TempFileCache.class.getClassLoader().hashCode())+", tempBaseDir "+tmpBaseDir.getAbsolutePath()); + } + + if(!staticInitError) { + try { + initTmpRoot(); + } catch (Exception ex) { + System.err.println("Warning: Catched Exception due to initializing TmpRoot:"); + ex.printStackTrace(); + staticInitError = true; + } + } + if (DEBUG) { + System.err.println("------------------------------------------------------------------ OK: "+(!staticInitError)); + } + } } /** @@ -96,8 +121,9 @@ public class TempFileCache { * 1. Synchronize on a global lock. Note that for this purpose we will * use System.out in the absence of a true global lock facility. * We are careful not to hold this lock too long. + * The global lock is claimed in the static initializer block, calling this method! * - * 2. Check for the existence of the "jnlp.applet.launcher.tmproot" + * 2. Check for the existence of the "jnlp.jogamp.tmp.cache.root" * system property. * * a. If set, then some other thread in a different ClassLoader has @@ -125,7 +151,7 @@ public class TempFileCache { * by that active process. This guarantee lets us avoid reaping an * active process' files. * - * 4. Set the "jnlp.applet.launcher.tmproot" system property. + * 4. Set the "jnlp.jogamp.tmp.cache.root" system property. * * 5. Add a shutdown hook to cleanup jlnNNNN.lck and jlnNNNN.tmp. We * don't actually expect that this shutdown hook will ever be called, @@ -137,117 +163,101 @@ public class TempFileCache { * 6. Start the Reaper thread to cleanup old installations. */ private static void initTmpRoot() throws IOException { - if (DEBUG) { - System.err.println("TempFileCache: Static Initialization ----------------------------------------------"); - System.err.println("TempFileCache: Thread: "+Thread.currentThread().getName()+", CL 0x"+Integer.toHexString(TempFileCache.class.getClassLoader().hashCode())); - } + tmpRootPropValue = System.getProperty(tmpRootPropName); - synchronized (System.out) { - // Get the name of the tmpbase directory. - tmpBaseDir = new File(IOUtil.getTempRoot(), tmpDirPrefix); - - tmpRootPropValue = System.getProperty(tmpRootPropName); - - if (tmpRootPropValue == null) { - // Create the tmpbase directory if it doesn't already exist - tmpBaseDir.mkdir(); - if (!tmpBaseDir.isDirectory()) { - throw new IOException("Cannot create directory " + tmpBaseDir); - } - - // Create ${tmpbase}/jlnNNNN.tmp then lock the file - File tmpFile = File.createTempFile("jln", ".tmp", tmpBaseDir); - if (DEBUG) { - System.err.println("TempFileCache: tmpFile = " + tmpFile.getAbsolutePath()); - } - final FileOutputStream tmpOut = new FileOutputStream(tmpFile); - final FileChannel tmpChannel = tmpOut.getChannel(); - final FileLock tmpLock = tmpChannel.lock(); - - // Strip off the ".tmp" to get the name of the tmprootdir - String tmpFileName = tmpFile.getAbsolutePath(); - String tmpRootName = tmpFileName.substring(0, tmpFileName.lastIndexOf(".tmp")); - - // create ${tmpbase}/jlnNNNN.lck then lock the file - String lckFileName = tmpRootName + ".lck"; - File lckFile = new File(lckFileName); - if (DEBUG) { - System.err.println("TempFileCache: lckFile = " + lckFile.getAbsolutePath()); - } - lckFile.createNewFile(); - final FileOutputStream lckOut = new FileOutputStream(lckFile); - final FileChannel lckChannel = lckOut.getChannel(); - final FileLock lckLock = lckChannel.lock(); - - // Create tmprootdir - tmpRootDir = new File(tmpRootName); - if (DEBUG) { - System.err.println("TempFileCache: tmpRootDir = " + tmpRootDir.getAbsolutePath()); - } - if (!tmpRootDir.mkdir()) { - throw new IOException("Cannot create " + tmpRootDir); - } + if (tmpRootPropValue == null) { + // Create ${tmpbase}/jlnNNNN.tmp then lock the file + File tmpFile = File.createTempFile("jln", ".tmp", tmpBaseDir); + if (DEBUG) { + System.err.println("TempFileCache: tmpFile = " + tmpFile.getAbsolutePath()); + } + final FileOutputStream tmpOut = new FileOutputStream(tmpFile); + final FileChannel tmpChannel = tmpOut.getChannel(); + final FileLock tmpLock = tmpChannel.lock(); + + // Strip off the ".tmp" to get the name of the tmprootdir + String tmpFileName = tmpFile.getAbsolutePath(); + String tmpRootName = tmpFileName.substring(0, tmpFileName.lastIndexOf(".tmp")); + + // create ${tmpbase}/jlnNNNN.lck then lock the file + String lckFileName = tmpRootName + ".lck"; + File lckFile = new File(lckFileName); + if (DEBUG) { + System.err.println("TempFileCache: lckFile = " + lckFile.getAbsolutePath()); + } + lckFile.createNewFile(); + final FileOutputStream lckOut = new FileOutputStream(lckFile); + final FileChannel lckChannel = lckOut.getChannel(); + final FileLock lckLock = lckChannel.lock(); + + // Create tmprootdir + tmpRootDir = new File(tmpRootName); + if (DEBUG) { + System.err.println("TempFileCache: tmpRootDir = " + tmpRootDir.getAbsolutePath()); + } + if (!tmpRootDir.mkdir()) { + throw new IOException("Cannot create " + tmpRootDir); + } - // Add shutdown hook to cleanup the OutputStream, FileChannel, - // and FileLock for the jlnNNNN.lck and jlnNNNN.lck files. - // We do this so that the locks never get garbage-collected. - Runtime.getRuntime().addShutdownHook(new Thread() { - /* @Override */ - public void run() { - // NOTE: we don't really expect that this code will ever - // be called. If it does, we will close the output - // stream, which will in turn close the channel. - // We will then release the lock. - try { - tmpOut.close(); - tmpLock.release(); - lckOut.close(); - lckLock.release(); - } catch (IOException ex) { - // Do nothing - } + // Add shutdown hook to cleanup the OutputStream, FileChannel, + // and FileLock for the jlnNNNN.lck and jlnNNNN.lck files. + // We do this so that the locks never get garbage-collected. + Runtime.getRuntime().addShutdownHook(new Thread() { + /* @Override */ + public void run() { + // NOTE: we don't really expect that this code will ever + // be called. If it does, we will close the output + // stream, which will in turn close the channel. + // We will then release the lock. + try { + tmpOut.close(); + tmpLock.release(); + lckOut.close(); + lckLock.release(); + } catch (IOException ex) { + // Do nothing } - }); - - // Set the system property... - tmpRootPropValue = tmpRootName.substring(tmpRootName.lastIndexOf(File.separator) + 1); - System.setProperty(tmpRootPropName, tmpRootPropValue); - if (DEBUG) { - System.err.println("TempFileCache: Setting " + tmpRootPropName + "=" + tmpRootPropValue); } + }); - // Start a new Reaper thread to do stuff... - Thread reaperThread = new Thread() { - /* @Override */ - public void run() { - deleteOldTempDirs(); - } - }; - reaperThread.setName("TempFileCache-Reaper"); - reaperThread.start(); - } else { - // Make sure that the property is not set to an illegal value - if (tmpRootPropValue.indexOf('/') >= 0 || - tmpRootPropValue.indexOf(File.separatorChar) >= 0) { - throw new IOException("Illegal value of: " + tmpRootPropName); - } + // Set the system property... + tmpRootPropValue = tmpRootName.substring(tmpRootName.lastIndexOf(File.separator) + 1); + System.setProperty(tmpRootPropName, tmpRootPropValue); + if (DEBUG) { + System.err.println("TempFileCache: Setting " + tmpRootPropName + "=" + tmpRootPropValue); + } - // Set tmpRootDir = ${tmpbase}/${jnlp.applet.launcher.tmproot} - if (DEBUG) { - System.err.println("TempFileCache: Using existing value of: " + - tmpRootPropName + "=" + tmpRootPropValue); - } - tmpRootDir = new File(tmpBaseDir, tmpRootPropValue); - if (DEBUG) { - System.err.println("TempFileCache: tmpRootDir = " + tmpRootDir.getAbsolutePath()); - } - if (!tmpRootDir.isDirectory()) { - throw new IOException("Cannot access " + tmpRootDir); + // Start a new Reaper thread to do stuff... + Thread reaperThread = new Thread() { + /* @Override */ + public void run() { + deleteOldTempDirs(); } + }; + reaperThread.setName("TempFileCache-Reaper"); + reaperThread.start(); + } else { + // Make sure that the property is not set to an illegal value + if (tmpRootPropValue.indexOf('/') >= 0 || + tmpRootPropValue.indexOf(File.separatorChar) >= 0) { + throw new IOException("Illegal value of: " + tmpRootPropName); + } + + // Set tmpRootDir = ${tmpbase}/${jnlp.applet.launcher.tmproot} + if (DEBUG) { + System.err.println("TempFileCache: Using existing value of: " + + tmpRootPropName + "=" + tmpRootPropValue); + } + tmpRootDir = new File(tmpBaseDir, tmpRootPropValue); + if (DEBUG) { + System.err.println("TempFileCache: tmpRootDir = " + tmpRootDir.getAbsolutePath()); + } + if (!tmpRootDir.isDirectory()) { + throw new IOException("Temp root directory does not exist: " + tmpRootDir.getAbsolutePath()); + } + if (!tmpRootDir.canWrite()) { + throw new IOException("Temp root directory is not writable: " + tmpRootDir.getAbsolutePath()); } - } - if (DEBUG) { - System.err.println("------------------------------------------------------------------ (static ok: "+(!staticInitError)+")"); } } @@ -412,13 +422,16 @@ public class TempFileCache { public boolean isValid() { return !staticInitError && !initError; } /** - * Base temp directory used by TempFileCache. - * Lifecycle: For all JVMs, ClassLoader and times. + * Base temp directory used by TempFileCache. + * + *

+ * Lifecycle: For one user's JVMs, ClassLoader and time. + *

* * This is set to: - * - * ${java.io.tmpdir}/ - * + * 
+     *   ${java.io.tmpdir}/tmpDirPrefix
+     *
* * @return */ @@ -428,14 +441,22 @@ public class TempFileCache { * Root temp directory for this JVM instance. Used to store individual * directories. * - * Lifecycle: For all JVMs and ClassLoader + *

+ * Lifecycle: For one user's concurrently running JVMs and ClassLoader + *

* - * / + * 
+     *   tmpBaseDir/tmpRootPropValue
+     *
* - * Use Case: Per ClassLoader files, eg. native libraries. + *

+ * Use Case: Per ClassLoader files, eg. native libraries. + *

* + *

* Old temp directories are cleaned up the next time a JVM is launched that * uses TempFileCache. + *

* * * @return @@ -446,9 +467,13 @@ public class TempFileCache { * Temporary directory for individual files (eg. native libraries of one ClassLoader instance). * The directory name is: * + *

* Lifecycle: Within each JVM .. use case dependent, ie. per ClassLoader + *

* - * /jlnMMMMM + * 
+     *   tmpRootDir/jlnMMMMM
+     *
* * where jlnMMMMM is the unique filename created by File.createTempFile() * without the ".tmp" extension. @@ -467,7 +492,6 @@ public class TempFileCache { * We avoid deleteOnExit, because it doesn't work reliably. */ private void createTmpDir() throws IOException { - File tmpFile = File.createTempFile("jln", ".tmp", tmpRootDir); String tmpFileName = tmpFile.getAbsolutePath(); String tmpDirName = tmpFileName.substring(0, tmpFileName.lastIndexOf(".tmp")); diff --git a/src/java/com/jogamp/common/util/cache/TempJarCache.java b/src/java/com/jogamp/common/util/cache/TempJarCache.java index 71a12e7..2286d6c 100644 --- a/src/java/com/jogamp/common/util/cache/TempJarCache.java +++ b/src/java/com/jogamp/common/util/cache/TempJarCache.java @@ -99,10 +99,10 @@ public class TempJarCache { /** * - * @return true if this class has been initialized, ie. used, otherwise not. + * @return true if this class has been properly initialized, ie. is in use, otherwise false. */ public static boolean isInitialized() { - return isInit; + return isInit && !staticInitError; } /* package */ static void checkInitialized() { -- cgit v1.2.3