Mercurial > hg > release > thermostat-0.4

changeset 556:517f3d4796af

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix osgi cache sharing (PR1110). All thermostat commands fire up their own OSGi framework. However, they used to share the OSGi cache location $THERMOSTAT_HOME/osgi by setting the org.osgi.framework.storage config option on framework start-up. Since the cache contains information about installed bundles as well as about the current state of bundles this can be problematic. This patch fixes this by creating a temporary directory in $TERMOSTAT_HOME/osgi-cache which gets deleted once the VM shuts down. Note that the cache is supposed to speed up application-start-up-time. By deleting the osgi cache on VM shutdown we won't have that benefit. Very informal testing didn't show significant speed-ups for me. That being said, we might want to revisit this again if it becomes a problem. The nice thing about the temp directory is that users won't have to specify it on the command-line. I've also added the START_TRANSIENT option to bundle.start() calls so as to avoid auto-starting of bundles. Reviewed-by: vanaltj, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-August/002836.html PR1110
author Severin Gehwolf <sgehwolf@redhat.com>
date Mon, 20 Aug 2012 15:29:14 +0200
parents f7f315bd6a53
children 4e9f1b8d4463
files bundles/src/main/java/com/redhat/thermostat/bundles/impl/BundleLoader.java distribution/pom.xml main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java
diffstat 3 files changed, 43 insertions(+), 5 deletions(-) [+]
line wrap: on
line diff
--- a/bundles/src/main/java/com/redhat/thermostat/bundles/impl/BundleLoader.java	Mon Aug 20 18:22:27 2012 +0200
+++ b/bundles/src/main/java/com/redhat/thermostat/bundles/impl/BundleLoader.java	Mon Aug 20 15:29:14 2012 +0200
@@ -81,7 +81,9 @@
             if (printOSGiInfo) {
                 System.out.println("BundleLoader: starting bundle: \"" + bundle.getBundleId() + "\"");
             }
-            bundle.start();
+            // We don't want for the framework to set the auto-start bit. Thus, passing
+            // START_TRANSIENT explicitly
+            bundle.start(Bundle.START_TRANSIENT);
         }
     }
 
--- a/distribution/pom.xml	Mon Aug 20 18:22:27 2012 +0200
+++ b/distribution/pom.xml	Mon Aug 20 15:29:14 2012 +0200
@@ -167,7 +167,7 @@
                 <mkdir dir="${project.build.directory}/agent/logs" />
                 <mkdir dir="${project.build.directory}/agent/run" />
                 <mkdir dir="${project.build.directory}/client" />
-                <mkdir dir="${project.build.directory}/osgi" />
+                <mkdir dir="${project.build.directory}/osgi-cache" />
                 <mkdir dir="${project.build.directory}/libs/native" />
                 
                 <!--  also copy the native libraries -->
--- a/main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java	Mon Aug 20 18:22:27 2012 +0200
+++ b/main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java	Mon Aug 20 15:29:14 2012 +0200
@@ -43,6 +43,8 @@
 import java.io.InputStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -51,10 +53,10 @@
 import java.util.Properties;
 import java.util.ServiceLoader;
 
-import org.osgi.framework.launch.FrameworkFactory;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.Constants;
 import org.osgi.framework.launch.Framework;
+import org.osgi.framework.launch.FrameworkFactory;
 import org.osgi.util.tracker.ServiceTracker;
 
 import com.redhat.thermostat.bundles.OSGiRegistryService;
@@ -70,6 +72,8 @@
 
     private boolean printOSGiInfo;
     private String thermostatHome;
+    // The framework cache location; Must not be shared between apps!
+    private Path osgiCacheStorage;
 
     public FrameworkProvider(Configuration config) {
         printOSGiInfo = config.getPrintOSGiInfo();
@@ -139,6 +143,11 @@
                     if (printOSGiInfo) {
                         System.out.println(DEBUG_PREFIX + "OSGi framework has shut down.");
                     }
+                    recursivelyDeleteDirectory(osgiCacheStorage.toFile());
+                    if (printOSGiInfo) {
+                        System.out.println(DEBUG_PREFIX + "Removed OSGi cache directory: "
+                                + osgiCacheStorage.toFile().getAbsolutePath());
+                    }
                 } catch (Exception e) {
                     throw new RuntimeException("Error shutting down framework.", e);
                 }
@@ -147,15 +156,42 @@
 
     }
 
+    private void recursivelyDeleteDirectory(File directory) {
+        for (File file: directory.listFiles()) {
+            if (file.isDirectory()) {
+                recursivelyDeleteDirectory(file);
+            }
+            file.delete();
+        }
+        directory.delete();
+    }
+
     private Framework makeFramework() throws FileNotFoundException, IOException {
-        File thermostatBundleHome = new File(thermostatHome, "osgi");
+        File osgiCacheDir = new File(thermostatHome, "osgi-cache");
 
+        // Create temporary directory which will be used as cache for OSGi bundles. See
+        // http://www.osgi.org/javadoc/r4v43/core/org/osgi/framework/Constants.html#FRAMEWORK_STORAGE
+        // for details about what this location is used for.
+        // 
+        // Agent, swing gui client application must not use the same location as this tricks the framework
+        // into thinking that some bundles are installed and loaded when that might not actually be the case.
+        // Note that we do not specify the org.osgi.framework.storage.clean property and the default is to NOT
+        // clean the cache, which is when we might run into trouble if this location is shared. This
+        // temp directory will be deleted on VM shutdown.
+        // 
+        // This fixes Thermostat BZ 1110.
+        osgiCacheStorage = Files.createTempDirectory(osgiCacheDir.toPath(), null);
+        if (printOSGiInfo) {
+            System.out.println(DEBUG_PREFIX + "OSGi cache location: "
+                    + osgiCacheStorage.toFile().getAbsolutePath());
+        }
+        
         ServiceLoader<FrameworkFactory> loader = ServiceLoader.load(FrameworkFactory.class,
                 getClass().getClassLoader());
         Map<String, String> bundleConfigurations = new HashMap<String, String>();
         String extraPackages = getOSGiPublicPackages();
         bundleConfigurations.put(Constants.FRAMEWORK_SYSTEMPACKAGES_EXTRA, extraPackages);
-        bundleConfigurations.put(Constants.FRAMEWORK_STORAGE, thermostatBundleHome.getAbsolutePath());
+        bundleConfigurations.put(Constants.FRAMEWORK_STORAGE, osgiCacheStorage.toFile().getAbsolutePath());
         Iterator<FrameworkFactory> factories = loader.iterator();
         if (factories.hasNext()) {
             // we just want the first found