# HG changeset patch # User Omair Majid # Date 1376500271 14400 # Node ID 69692e087b1358bd3b86fa39537c6eb76d3b5dd8 # Parent d44f06cf8a581324dcfd0e8763cd6eb91d4fa557 PR1514: Thermostat should not start bundles with the same symbolic name/version twice Reviewed-by: jerboaa, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-August/007809.html diff -r d44f06cf8a58 -r 69692e087b13 integration-tests/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java Wed Aug 14 10:38:38 2013 -0400 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java Wed Aug 14 13:11:11 2013 -0400 @@ -296,11 +296,21 @@ } } + /** Confirm that there are no 'command not found'-like messages in the spawn's stdout/stderr */ + public static void assertCommandIsFound(Spawn spawn) { + assertCommandIsFound(spawn.getCurrentStandardOutContents(), spawn.getCurrentStandardErrContents()); + } + public static void assertCommandIsFound(String stdOutContents, String stdErrContents) { assertFalse(stdOutContents.contains("unknown command")); assertFalse(stdErrContents.contains("unknown command")); } + /** Confirm that there are no exception stack traces in the spawn's stdout/stderr */ + public static void assertNoExceptions(Spawn spawn) { + assertNoExceptions(spawn.getCurrentStandardOutContents(), spawn.getCurrentStandardErrContents()); + } + public static void assertNoExceptions(String stdOutContents, String stdErrContents) { assertFalse(stdOutContents.contains("Exception")); assertFalse(stdErrContents.contains("Exception")); diff -r d44f06cf8a58 -r 69692e087b13 integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Wed Aug 14 10:38:38 2013 -0400 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Wed Aug 14 13:11:11 2013 -0400 @@ -125,6 +125,26 @@ } } + @Test + public void testNormalCommandAndPluginInShell() throws Exception { + Spawn shell = spawnThermostat("shell"); + + shell.expect(SHELL_PROMPT); + shell.send("list-vms\n"); + + shell.expect(SHELL_PROMPT); + + shell.send("dump-heap\n"); + + shell.expect(SHELL_PROMPT); + + shell.send("exit\n"); + shell.expectClose(); + + assertCommandIsFound(shell); + assertNoExceptions(shell); + } + private static Spawn commandAgainstMongo(String... args) throws IOException { if (args == null || args.length == 0) { throw new IllegalArgumentException("args must be an array with something"); diff -r d44f06cf8a58 -r 69692e087b13 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java Wed Aug 14 10:38:38 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java Wed Aug 14 13:11:11 2013 -0400 @@ -37,6 +37,7 @@ package com.redhat.thermostat.launcher.internal; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -104,14 +105,21 @@ continue; } File file = new File(libRoot, value.trim()); - String path = file.toURI().toString(); - if (!file.exists()) { - logger.severe("Bundle " + path + " required by " + getName() + + try { + String path = file.getCanonicalFile().toURI().toString(); + if (!file.exists()) { + logger.severe("Bundle " + path + " required by " + getName() + + " command does not exist in the filesystem. This will cause" + + " osgi wiring issue when attempting to run this command."); + // Allow to proceed because this command may never be called. + } else { + dependencies.add(path); + } + } catch (IOException e) { + logger.severe("Bundle " + file + " required by " + getName() + " command does not exist in the filesystem. This will cause" + " osgi wiring issue when attempting to run this command."); // Allow to proceed because this command may never be called. - } else { - dependencies.add(path); } } } diff -r d44f06cf8a58 -r 69692e087b13 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java Wed Aug 14 10:38:38 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java Wed Aug 14 13:11:11 2013 -0400 @@ -72,7 +72,12 @@ private static final Logger logger = LoggingUtils.getLogger(BundleManagerImpl.class); - // Bundle Name and version -> path + // Bundle Name and version -> path (with symlinks resolved) + // Match FrameworkProvider which uses canonical/symlink-resolved paths. If + // there is a mismatch, there are going to be clashes with trying to load + // two files (which are really the same) with the same bundle symbolic + // name/version. See + // http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1514 private final Map known; private Configuration configuration; private BundleLoader loader; @@ -102,7 +107,17 @@ if (name == null || version == null) { logger.config("file " + file.toString() + " is missing osgi metadata; wont be usable for dependencies"); } else { - known.put(new BundleInformation(name, version), file); + BundleInformation info = new BundleInformation(name, version); + Path old = known.get(info); + // Path is completely resolved. First one wins. + if (old == null) { + known.put(info, file.toRealPath()); + } else { + if (!old.equals(file.toRealPath())) { + logger.warning("bundles " + old + " and " + file + " both provide " + info); + } + // leave old + } } } catch (IOException e) { logger.severe("Error in reading " + file); @@ -121,10 +136,10 @@ if (configuration.getPrintOSGiInfo()) { logger.fine("Found: " + known.size() + " bundles"); logger.fine("Took " + (t2 -t1) + "ns"); + } - for (Entry bundles : known.entrySet()) { - logger.finest(bundles.getKey().toString() + " is at " + bundles.getValue().toString()); - } + for (Entry bundles : known.entrySet()) { + logger.finest(bundles.getKey().toString() + " is at " + bundles.getValue().toString()); } } @@ -143,7 +158,7 @@ logger.warning("no known bundle matching " + info.toString()); continue; } - paths.add(bundlePath.toFile().toURI().toString()); + paths.add(bundlePath.toFile().getCanonicalFile().toURI().toString()); } loadBundlesByPath(paths); } diff -r d44f06cf8a58 -r 69692e087b13 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfo.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfo.java Wed Aug 14 10:38:38 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfo.java Wed Aug 14 13:11:11 2013 -0400 @@ -75,7 +75,10 @@ List getBundles(); - /** Returns a list of jar that this command depends on */ + /** + * Returns a list of jar that this command depends on + * @return a list of strings containing the URIs of jars-on-disk + */ List getDependencyResourceNames(); } diff -r d44f06cf8a58 -r 69692e087b13 main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java --- a/main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java Wed Aug 14 10:38:38 2013 -0400 +++ b/main/src/main/java/com/redhat/thermostat/main/impl/FrameworkProvider.java Wed Aug 14 13:11:11 2013 -0400 @@ -62,6 +62,7 @@ import com.redhat.thermostat.common.Launcher; import com.redhat.thermostat.launcher.BundleManager; import com.redhat.thermostat.shared.config.Configuration; +import com.redhat.thermostat.shared.config.InvalidConfigurationException; public class FrameworkProvider { @@ -283,8 +284,18 @@ } } + // Resolve symlinks completely; This makes the behaviour of this class + // consistent with BundleManagerImpl and avoid problems where two different + // files across bootstrap and later code provide the same bundle symbolic + // name version private String actualLocation(String resourceName) { - return new File(configuration.getSystemLibRoot(), resourceName).toURI().toString(); + File file = new File(configuration.getSystemLibRoot(), resourceName); + try { + return file.getCanonicalFile().toURI().toString(); + } catch (IOException e) { + // okay, lets not canonicalize the path + return file.toURI().toString(); + } } }