From d2fc21dc1dd8806429dde964bcfd6c2417a423bd Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Wed, 15 Jul 2015 08:44:10 -0700 Subject: [PATCH] Better support for split packages. - if you force an inclusion via 'inclusions=foo..*' then allow for everything in that package to be made reloadable regardless of it coming from jar or disk. - when inclusion patterns don't match fall back on regular acceptance mechanisms (like package cache, etc) rather than just dismissing it. - the results of decision making around making things reloadable are now enums/objects rather than plain booleans so that later we can do further analysis on those decisions. Issue: https://github.com/grails/grails-core/issues/8817 --- .../loaded/GlobalConfiguration.java | 2 + .../org/springsource/loaded/TypeRegistry.java | 159 ++++++++++++++---- .../agent/SpringLoadedPreProcessor.java | 34 +++- .../loaded/test/ReloadableTypeTests.java | 47 +++--- .../loaded/test/ReloadingJVM.java | 15 ++ .../test/ReloadingJVMCommandProcess.java | 19 +++ .../loaded/test/SpringLoadedTests.java | 3 + .../test/SpringLoadedTestsInSeparateJVM.java | 38 +++++ testdata/jars/grailsplugins/A.class | Bin 0 -> 258 bytes testdata/jars/grailsplugins/build.sh | 4 + .../jars/grailsplugins/grails/plugins/A.class | Bin 0 -> 485 bytes testdata/jars/grailsplugins/grailsplugins.jar | Bin 0 -> 486 bytes testdata/src/main/java/grails/plugins/A.java | 8 + testdata/src/main/java/grails/plugins/B.java | 8 + testdata/src/main/java/grails/plugins/B2.java | 8 + 15 files changed, 279 insertions(+), 66 deletions(-) create mode 100644 testdata/jars/grailsplugins/A.class create mode 100755 testdata/jars/grailsplugins/build.sh create mode 100644 testdata/jars/grailsplugins/grails/plugins/A.class create mode 100644 testdata/jars/grailsplugins/grailsplugins.jar create mode 100644 testdata/src/main/java/grails/plugins/A.java create mode 100644 testdata/src/main/java/grails/plugins/B.java create mode 100644 testdata/src/main/java/grails/plugins/B2.java diff --git a/springloaded/src/main/java/org/springsource/loaded/GlobalConfiguration.java b/springloaded/src/main/java/org/springsource/loaded/GlobalConfiguration.java index 255378e..346e453 100644 --- a/springloaded/src/main/java/org/springsource/loaded/GlobalConfiguration.java +++ b/springloaded/src/main/java/org/springsource/loaded/GlobalConfiguration.java @@ -450,6 +450,8 @@ public class GlobalConfiguration { public final static boolean isJava18orHigher; + public static boolean InTestMode = false; + static { String version = System.getProperty("java.version"); if (version.startsWith("1.8")) { diff --git a/springloaded/src/main/java/org/springsource/loaded/TypeRegistry.java b/springloaded/src/main/java/org/springsource/loaded/TypeRegistry.java index 0947a05..487a4d5 100644 --- a/springloaded/src/main/java/org/springsource/loaded/TypeRegistry.java +++ b/springloaded/src/main/java/org/springsource/loaded/TypeRegistry.java @@ -601,6 +601,33 @@ public class TypeRegistry { private List packagesNotFound = new ArrayList(); + + public static enum CouldBeReloadableDecision { + No_BuiltIn(false, false, "built in rejection"), // + No_FixedPackageList(false, false, "on hard coded list of those to reject"), // + Yes_CGLIB(true, false, "cglib related type"), // + No_JSP(false, false, "jsp"), // + No_GroovyScript(false, false, "groovy script"), // + Yes_PackageCache(true, false, "package cache"), // + No_PackageCache(false, false, "package cache"), // + Yes_FoundInJar(true, true, "in jar"), // + Yes_FoundOnDisk(true, true, "on disk"), // + No_Array(false, false, "array"), // + No_DiskCheck(false, true, "disk checked"); + + public final boolean couldBeReloadable; + + public final boolean diskChecked; + + public String reason; + + CouldBeReloadableDecision(boolean couldBeReloadable, boolean diskChecked, String reason) { + this.couldBeReloadable = couldBeReloadable; + this.diskChecked = diskChecked; + this.reason = reason; + } + } + /** * Determine if the named type could be reloadable. This method is invoked if the user has not setup any inclusions. * With no inclusions specified, something is considered reloadable if it is accessible by the classloader for this @@ -609,12 +636,12 @@ public class TypeRegistry { * @param slashedName the typename of interest (e.g. com/foo/Bar) * @return true if the type should be considered reloadable */ - private boolean couldBeReloadable(String slashedName) { + private CouldBeReloadableDecision couldBeReloadable(String slashedName) { if (slashedName == null) { - return false; + return CouldBeReloadableDecision.No_BuiltIn; } if (slashedName.startsWith("java/")) { - return false; + return CouldBeReloadableDecision.No_BuiltIn; } char ch = slashedName.charAt(0); int index = ch - 'a'; @@ -630,27 +657,27 @@ public class TypeRegistry { + ignorablePackagePrefix + "' which is considered infrastructure and types within it are not made reloadable"); } - return false; + return CouldBeReloadableDecision.No_FixedPackageList; } } } } if (slashedName.indexOf("$Proxy") != -1 || slashedName.indexOf("$$EnhancerBy") != -1 || slashedName.indexOf("$$FastClassBy") != -1) { - return true; + return CouldBeReloadableDecision.Yes_CGLIB; } // TODO review all these... are these four only loaded by jasperloader? int underscorePos = slashedName.indexOf("_"); if (underscorePos != -1) { if (slashedName.endsWith("_jspx") || slashedName.endsWith("_tagx")) { - return false; + return CouldBeReloadableDecision.No_JSP; } if (slashedName.endsWith("_jspx$Helper") || slashedName.endsWith("_tagx$Helper")) { - return false; + return CouldBeReloadableDecision.No_JSP; } // skip grails scripts like "_PackagePlugins_groovy$_run_closure1_closure7" if (ch == '_' && slashedName.indexOf("_groovy") != -1) { - return false; + return CouldBeReloadableDecision.No_GroovyScript; } } int lastSlashPos = slashedName.lastIndexOf('/'); @@ -660,18 +687,18 @@ public class TypeRegistry { for (String foundPackageName : packagesFound) { if (packageName.equals(foundPackageName)) { // System.out.println("fast accept " + slashedName); - return true; + return CouldBeReloadableDecision.Yes_PackageCache; } } for (String notfoundPackageName : packagesNotFound) { if (packageName.equals(notfoundPackageName)) { // System.out.println("fast reject " + slashedName); - return false; + return CouldBeReloadableDecision.No_PackageCache; } } } if (ch == '[') { - return false; + return CouldBeReloadableDecision.No_Array; } try { if (getResourceMethod == null) { @@ -685,6 +712,7 @@ public class TypeRegistry { getResourceMethod.setAccessible(true); URL url = (URL) getResourceMethod.invoke(classLoader.get(), slashedName + ".class"); boolean reloadable = false; + boolean jarEntry = false; if (url != null) { String protocol = url.getProtocol(); // ignore 'jar' - what others? @@ -708,6 +736,7 @@ public class TypeRegistry { for (String jarToWatch : GlobalConfiguration.jarsToWatch) { if (jarname.equals(jarToWatch)) { reloadable = true; + jarEntry = true; } } } @@ -725,7 +754,17 @@ public class TypeRegistry { // System.out.println("expensive, no package name and URL checked: " + slashedName + " : " + url + " loader=" // + classLoader); } - return reloadable; + if (reloadable) { + if (jarEntry) { + return CouldBeReloadableDecision.Yes_FoundInJar; + } + else { + return CouldBeReloadableDecision.Yes_FoundOnDisk; + } + } + else { + return CouldBeReloadableDecision.No_DiskCheck; + } } catch (Exception e) { throw new ReloadException("Unexpected problem locating the bytecode for " + slashedName + ".class", e); @@ -733,7 +772,36 @@ public class TypeRegistry { } public boolean isReloadableTypeName(String slashedName) { - return isReloadableTypeName(slashedName, null, null); + return isReloadableTypeName(slashedName, null, null).isReloadable; + } + + public static class ReloadableTypeNameDecision { + + public final boolean isReloadable; + + public final boolean extraInfo; + + public final CouldBeReloadableDecision cbrd; + + public final String reason; + + public final boolean explicitlyIncluded; + + ReloadableTypeNameDecision(boolean reloadable, CouldBeReloadableDecision cbrd, String reason, boolean extraInfo, + boolean explicitlyIncluded) { + this.isReloadable = reloadable; + this.cbrd = cbrd; + this.reason = reason; + this.extraInfo = extraInfo; + this.explicitlyIncluded = explicitlyIncluded; + } + + public static final ReloadableTypeNameDecision No = new ReloadableTypeNameDecision(false, null, + null, false, false); + + public static final ReloadableTypeNameDecision Yes = new ReloadableTypeNameDecision(true, null, + null, false, false); + } /** @@ -745,7 +813,8 @@ public class TypeRegistry { * @param bytes the class bytes for the class being loaded * @return true if the type is reloadable, false otherwise */ - public boolean isReloadableTypeName(String slashedName, ProtectionDomain protectionDomain, byte[] bytes) { + public ReloadableTypeNameDecision isReloadableTypeName(String slashedName, ProtectionDomain protectionDomain, + byte[] bytes) { if (GlobalConfiguration.verboseMode && log.isLoggable(Level.FINER)) { log.finer("entering TypeRegistry.isReloadableTypeName(" + slashedName + ")"); } @@ -758,7 +827,7 @@ public class TypeRegistry { log.finer("[explanation] The type " + slashedName + " is considered part of yourkit and is not being made reloadable"); } - return false; + return ReloadableTypeNameDecision.No; } } // Proxy types that implement a reloadable interface should themselves be made reloadable ... to be fleshed out @@ -788,34 +857,35 @@ public class TypeRegistry { log.finer("[explanation] The plugin " + plugin.getClass().getName() + " determined type " + slashedName + " is reloadable"); } - return true; + return ReloadableTypeNameDecision.Yes; } else if (decision == ReloadDecision.NO) { if (GlobalConfiguration.explainMode && log.isLoggable(Level.FINER)) { log.finer("[explanation] The plugin " + plugin.getClass().getName() + " determined type " + slashedName + " is not reloadable"); } - return false; + return ReloadableTypeNameDecision.No; } } if (inclusionPatterns.isEmpty()) { // No inclusions, so unless it matches an exclusion, it will be included if (exclusionPatterns.isEmpty()) { - if (couldBeReloadable(slashedName)) { + CouldBeReloadableDecision cbrd = couldBeReloadable(slashedName); + if (cbrd.couldBeReloadable) { if (GlobalConfiguration.explainMode && log.isLoggable(Level.FINER)) { log.finer("[explanation] The class " + slashedName + " is currently considered reloadable. It matches no exclusions, is accessible from this classloader and is not in a jar/zip."); } - return true; + return new ReloadableTypeNameDecision(true, cbrd, null, true, false); } else { if (GlobalConfiguration.explainMode && log.isLoggable(Level.FINER)) { log.finer("[explanation] The class " + slashedName + " is not going to be treated as reloadable."); } - return false; + return new ReloadableTypeNameDecision(false, cbrd, null, true, false); } } else { @@ -829,14 +899,15 @@ public class TypeRegistry { } } if (isExcluded) { - return false; + return new ReloadableTypeNameDecision(false, null, null, false, false); } } - if (couldBeReloadable(slashedName)) { - return true; + CouldBeReloadableDecision cbrd = couldBeReloadable(slashedName); + if (cbrd.couldBeReloadable) { + return new ReloadableTypeNameDecision(true, cbrd, null, false, false); } else { - return false; + return new ReloadableTypeNameDecision(false, cbrd, null, false, false); } } } @@ -853,12 +924,23 @@ public class TypeRegistry { } } if (!isIncluded) { - return false; + // Not on the inclusion list + // In test mode there are various hierarchies of test data classes all on disk + // but inclusions are used to specify exactly what we want to consider reloadable. By + // making this check we avoid making types we discover on disk (or in the package cache) + // being made reloadable. In a real setup this wouldn't be what we want (hence the check) + if (!GlobalConfiguration.InTestMode) { + CouldBeReloadableDecision cbrd = couldBeReloadable(slashedName); + if (cbrd.couldBeReloadable) { + return new ReloadableTypeNameDecision(true, cbrd, null, true, false); + } + } + return ReloadableTypeNameDecision.No; } } // Ok it matched an inclusion, but it must not match any exclusions if (exclusionPatterns.isEmpty()) { - return true; + return new ReloadableTypeNameDecision(true, null, null, true, true); } else { boolean isExcluded = false; @@ -870,7 +952,7 @@ public class TypeRegistry { } } } - return !isExcluded; + return new ReloadableTypeNameDecision(!isExcluded, null, null, true, true); } } } @@ -1058,7 +1140,8 @@ public class TypeRegistry { } } if (GlobalConfiguration.logging && log.isLoggable(Level.INFO)) { - log.log(Level.INFO, "ReloadableType.addType(): Type '" + dottedname + "' is now reloadable! id=" + typeId); + log.log(Level.INFO, + "ReloadableType.addType(): Type '" + dottedname + "' is now reloadable! id=" + typeId); } return rtype; } @@ -1207,7 +1290,8 @@ public class TypeRegistry { defineClassMethod.setAccessible(true); ClassLoader loaderToUse = null; loaderToUse = classLoader.get(); - clazz = (Class) defineClassMethod.invoke(loaderToUse, new Object[] { name, bytes, 0, bytes.length }); + clazz = (Class) defineClassMethod.invoke(loaderToUse, + new Object[] { name, bytes, 0, bytes.length }); } else { clazz = ccl.defineClass(name, bytes); @@ -1344,8 +1428,9 @@ public class TypeRegistry { } else if (IncrementalTypeDescriptor.hasChanged(method)) { if (IncrementalTypeDescriptor.isNowNonStatic(method)) { - throw new IncompatibleClassChangeError("SpringLoaded: Target of static call is no longer static '" - + reloadableType.getBaseName() + "." + nameAndDescriptor + "'"); + throw new IncompatibleClassChangeError( + "SpringLoaded: Target of static call is no longer static '" + + reloadableType.getBaseName() + "." + nameAndDescriptor + "'"); } // TODO need a check in here for a visibility change? Something like this: // if (IncrementalTypeDescriptor.hasVisibilityChanged(method)) { @@ -1474,7 +1559,8 @@ public class TypeRegistry { */ private static ReloadableType searchForReloadableType(int typeId, TypeRegistry typeRegistry) { ReloadableType reloadableType; - reloadableType = typeRegistry.getReloadableTypeInTypeRegistryHierarchy(NameRegistry.getTypenameById(typeId)); + reloadableType = typeRegistry.getReloadableTypeInTypeRegistryHierarchy( + NameRegistry.getTypenameById(typeId)); typeRegistry.rememberReloadableType(typeId, reloadableType); return reloadableType; } @@ -1802,7 +1888,8 @@ public class TypeRegistry { @UsedByGeneratedCode public static ReloadableType getReloadableType(int typeRegistryId, int typeId) { if (GlobalConfiguration.verboseMode && log.isLoggable(Level.INFO)) { - log.info(">TypeRegistry.getReloadableType(typeRegistryId=" + typeRegistryId + ",typeId=" + typeId + ")"); + log.info( + ">TypeRegistry.getReloadableType(typeRegistryId=" + typeRegistryId + ",typeId=" + typeId + ")"); } TypeRegistry typeRegistry = registryInstances[typeRegistryId].get(); if (typeRegistry == null) { @@ -1919,14 +2006,16 @@ public class TypeRegistry { // 'local' plugins for (Plugin plugin : localPlugins) { if (plugin instanceof ReloadEventProcessorPlugin) { - ((ReloadEventProcessorPlugin) plugin).reloadEvent(reloadableType.getName(), reloadableType.getClazz(), + ((ReloadEventProcessorPlugin) plugin).reloadEvent(reloadableType.getName(), + reloadableType.getClazz(), versionsuffix); } } // 'global' plugins for (Plugin plugin : SpringLoadedPreProcessor.getGlobalPlugins()) { if (plugin instanceof ReloadEventProcessorPlugin) { - ((ReloadEventProcessorPlugin) plugin).reloadEvent(reloadableType.getName(), reloadableType.getClazz(), + ((ReloadEventProcessorPlugin) plugin).reloadEvent(reloadableType.getName(), + reloadableType.getClazz(), versionsuffix); } } diff --git a/springloaded/src/main/java/org/springsource/loaded/agent/SpringLoadedPreProcessor.java b/springloaded/src/main/java/org/springsource/loaded/agent/SpringLoadedPreProcessor.java index cdb27ef..99f3ae3 100644 --- a/springloaded/src/main/java/org/springsource/loaded/agent/SpringLoadedPreProcessor.java +++ b/springloaded/src/main/java/org/springsource/loaded/agent/SpringLoadedPreProcessor.java @@ -42,6 +42,7 @@ import org.springsource.loaded.SystemClassReflectionInvestigator; import org.springsource.loaded.SystemClassReflectionRewriter; import org.springsource.loaded.SystemClassReflectionRewriter.RewriteResult; import org.springsource.loaded.TypeRegistry; +import org.springsource.loaded.TypeRegistry.ReloadableTypeNameDecision; import org.springsource.loaded.Utils; import org.springsource.loaded.ri.ReflectiveInterceptor; import org.springsource.loaded.support.Java8; @@ -188,9 +189,10 @@ public class SpringLoadedPreProcessor implements Constants { // 2. If NO, and nothing in this classloader might be, return the original bytes. // 3. If YES, make the type reloadable (including rewriting call sites) - boolean isReloadableTypeName = typeRegistry.isReloadableTypeName(slashedClassName, protectionDomain, bytes); + ReloadableTypeNameDecision isReloadableTypeName = typeRegistry.isReloadableTypeName(slashedClassName, + protectionDomain, bytes); - if (isReloadableTypeName && GlobalConfiguration.explainMode && log.isLoggable(Level.INFO)) { + if (isReloadableTypeName.isReloadable && GlobalConfiguration.explainMode && log.isLoggable(Level.INFO)) { log.info("[explanation] Based on the name, type " + slashedClassName + " is considered to be reloadable"); } @@ -199,7 +201,7 @@ public class SpringLoadedPreProcessor implements Constants { // if (GlobalConfiguration.verboseMode && isReloadableTypeName) { // Log.log("Type '"+slashedClassName+"' is preliminarily being considered a reloadable type"); // } - if (isReloadableTypeName) { + if (isReloadableTypeName.isReloadable) { if (!firstReloadableTypeHit) { firstReloadableTypeHit = true; // TODO move into the ctor for ReloadableType so that it can't block loading @@ -305,13 +307,29 @@ public class SpringLoadedPreProcessor implements Constants { if (!makeReloadableAnyway) { // can't watch it for updates (it comes from a jar perhaps) so just rewrite call sites and return - if (GlobalConfiguration.verboseMode) { - Log.log("Cannot watch " + slashedClassName + ": not making it reloadable"); + + // Not planning to watch this class so ordinarily do not make it reloadable. UNLESS the user + // is specifying that it needs to be. This may happen with split packages - some classes in a jar + // and some on disk. During type rewriting the top most reloadable types get fields inserted - + // when split across jars we get confused by split packages. If we go by name (as the code does + // right now) then we think we aren't the top most reloadable type but we don't realize that the + // type above us comes from a jar. Hence this condition below. If the user did explicitly + // specify types with this kind of name should be made reloadable we even make the ones from + // the jar reloadable. (TODO: optimization, make a smarter isTopMostReloadableType test that + // allows us to keep the jar loaded types as non reloadable). + if (isReloadableTypeName.extraInfo && isReloadableTypeName.explicitlyIncluded + && !GlobalConfiguration.InTestMode) { + } - if (needsClientSideRewriting(slashedClassName)) { - bytes = typeRegistry.methodCallRewriteUseCacheIfAvailable(slashedClassName, bytes); + else { + if (GlobalConfiguration.verboseMode) { + Log.log("Cannot watch " + slashedClassName + ": not making it reloadable"); + } + if (needsClientSideRewriting(slashedClassName)) { + bytes = typeRegistry.methodCallRewriteUseCacheIfAvailable(slashedClassName, bytes); + } + return bytes; } - return bytes; } } ReloadableType rtype = typeRegistry.addType(dottedClassName, bytes); diff --git a/springloaded/src/test/java/org/springsource/loaded/test/ReloadableTypeTests.java b/springloaded/src/test/java/org/springsource/loaded/test/ReloadableTypeTests.java index 40797d6..a4c0320 100644 --- a/springloaded/src/test/java/org/springsource/loaded/test/ReloadableTypeTests.java +++ b/springloaded/src/test/java/org/springsource/loaded/test/ReloadableTypeTests.java @@ -32,15 +32,13 @@ import org.springsource.loaded.GlobalConfiguration; import org.springsource.loaded.ReloadableType; import org.springsource.loaded.SpringLoaded; import org.springsource.loaded.TypeRegistry; -import org.springsource.loaded.Utils; import org.springsource.loaded.Utils.ReturnType; -import org.springsource.loaded.test.infra.ClassPrinter; import org.springsource.loaded.test.infra.Result; /** * Tests for the TypeRegistry that exercise it in the same way it will actively be used when managing ReloadableType * instances. - * + * * @author Andy Clement * @since 1.0 */ @@ -136,7 +134,8 @@ public class ReloadableTypeTests extends SpringLoadedTests { assertEquals("[1,2,3]", toString(runOnInstance(rtype.getClazz(), instance, "getProtectedArrayOfInts").returnValue)); - runOnInstance(rtype.getClazz(), instance, "setProtectedArrayOfStrings", (Object) new String[] { "a", "b", "c" }); + runOnInstance(rtype.getClazz(), instance, "setProtectedArrayOfStrings", + (Object) new String[] { "a", "b", "c" }); assertEquals("[a,b,c]", toString(runOnInstance(rtype.getClazz(), instance, "getProtectedArrayOfStrings").returnValue)); @@ -182,7 +181,8 @@ public class ReloadableTypeTests extends SpringLoadedTests { assertEquals("[1,2,3]", toString(runOnInstance(rtype.getClazz(), instance, "getProtectedArrayOfInts").returnValue)); - runOnInstance(rtype.getClazz(), instance, "setProtectedArrayOfStrings", (Object) new String[] { "a", "b", "c" }); + runOnInstance(rtype.getClazz(), instance, "setProtectedArrayOfStrings", + (Object) new String[] { "a", "b", "c" }); assertEquals("[a,b,c]", toString(runOnInstance(rtype.getClazz(), instance, "getProtectedArrayOfStrings").returnValue)); @@ -206,13 +206,13 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType B = tr.addType("invokestatic.issue4.B", loadBytesForClass("invokestatic.issue4.B")); Result r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); B.loadNewVersion(B.bytesInitial); r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); } @Test @@ -222,12 +222,12 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType BB = tr.addType("invokestatic.issue4.BB", loadBytesForClass("invokestatic.issue4.BB")); Result r = runUnguarded(BB.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); BB.loadNewVersion(BB.bytesInitial); r = runUnguarded(BB.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); } @Test @@ -237,12 +237,12 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType BBB = tr.addType("invokestatic.issue4.BBB", loadBytesForClass("invokestatic.issue4.BBB")); Result r = runUnguarded(BBB.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); AAA.loadNewVersion(AAA.bytesInitial); r = runUnguarded(BBB.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); } @Test @@ -252,13 +252,13 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType B = tr.addType("invokestatic.issue4.B", loadBytesForClass("invokestatic.issue4.B")); Result r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); A.loadNewVersion(A.bytesInitial); B.loadNewVersion(B.bytesInitial); r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("String1", (String) r.returnValue); + assertEquals("String1", r.returnValue); } // The supertype is not reloadable,it is in a jar @@ -268,7 +268,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType B = tr.addType("invokestatic.issue4.BBBB", loadBytesForClass("invokestatic.issue4.BBBB")); Result r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); ReloadableType thesuper = B.getSuperRtype(); assertNull(thesuper); @@ -278,7 +278,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { B.loadNewVersion(B.bytesInitial); r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); } // Basic write/read then reload then write/read again @@ -366,7 +366,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { // There is a test that will work in the SpringLoadedTestsInSeparateJVM public void serialization4() throws Exception { TypeRegistry tr = getTypeRegistry("remote..*"); - // ReloadableType person = + // ReloadableType person = tr.addType("remote.Person", loadBytesForClass("remote.Person")); // When the Serialize class is run directly, we see: byteinfo:len=98:crc=c1047cf6 @@ -386,7 +386,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType B = tr.addType("invokestatic.issue4.BBBBB", loadBytesForClass("invokestatic.issue4.BBBBB")); Result r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); ReloadableType thesuper = B.getSuperRtype(); thesuper = tr.getReloadableType("invokestatic/issue4/subpkg/AAAA"); @@ -395,7 +395,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { B.loadNewVersion(B.bytesInitial); r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); } @Test @@ -405,7 +405,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { ReloadableType B = tr.addType("invokestatic.issue4.BBBBB", loadBytesForClass("invokestatic.issue4.BBBBB")); Result r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); ReloadableType thesuper = B.getSuperRtype(); thesuper = tr.getReloadableType("invokestatic/issue4/subpkg/AAAA"); @@ -414,7 +414,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { B.loadNewVersion(B.bytesInitial); r = runUnguarded(B.getClazz(), "getMessage"); - assertEquals("Hello", (String) r.returnValue); + assertEquals("Hello", r.returnValue); assertAssociateSubtypes(mid, "invokestatic.issue4.BBBBB"); assertTrue(B.isAffectedByReload()); @@ -425,7 +425,7 @@ public class ReloadableTypeTests extends SpringLoadedTests { /** * Verify that the reloadable type knows about the expected list of subtypes. Subtype names are dotted (e.g. * invokestatic.foo.Bar") - * + * * @param rtype the reloadabletype to check * @param expectedAssociateSubtypes the subtypes it should know about */ @@ -485,12 +485,12 @@ public class ReloadableTypeTests extends SpringLoadedTests { /** * In this test a protected field has the same name as another field being referenced from the reloadable type. * Check only the right one is redirect to the accessor. - * + * */ @Test public void protectedFieldAccessors3() throws Exception { TypeRegistry tr = getTypeRegistry("prot.SubThree,prot.PeerThree"); - // ReloadableType rtypePeer = + // ReloadableType rtypePeer = tr.addType("prot.PeerThree", loadBytesForClass("prot.PeerThree")); ReloadableType rtype = tr.addType("prot.SubThree", loadBytesForClass("prot.SubThree")); @@ -653,4 +653,5 @@ public class ReloadableTypeTests extends SpringLoadedTests { assertTrue(Modifier.isPublic((Integer) runUnguarded(simpleClass, "getModifiers").returnValue)); assertTrue(Modifier.isStatic((Integer) runUnguarded(simpleClass, "getModifiers").returnValue)); } + } diff --git a/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVM.java b/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVM.java index eff1c5f..cdfc212 100644 --- a/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVM.java +++ b/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVM.java @@ -378,6 +378,21 @@ public class ReloadingJVM { } } + public boolean isReloadableType(String classname) { + JVMOutput jo = sendAndReceive("isreloadabletype " + classname); + System.out.println(jo); + if (jo.stdout.contains(classname + " is reloadable type")) { + return true; + } + else if (jo.stdout.contains(classname + " is not reloadable type")) { + return false; + } + else { + System.err.println(jo); + throw new IllegalStateException(); + } + } + public JVMOutput newInstance(String instanceName, String classname) { return newInstance(instanceName, classname, true); } diff --git a/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVMCommandProcess.java b/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVMCommandProcess.java index 5b3cc6f..8a796a3 100644 --- a/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVMCommandProcess.java +++ b/springloaded/src/test/java/org/springsource/loaded/test/ReloadingJVMCommandProcess.java @@ -98,6 +98,9 @@ public class ReloadingJVMCommandProcess { else if (commandName.equals("reload")) { reloadCommand(arguments.get(0), arguments.size() == 1 ? null : arguments.get(1)); } + else if (commandName.equals("isreloadabletype")) { + isReloadableTypeCheck(arguments.get(0)); + } else if (commandName.equals("extendcp")) { extendClasspath(arguments.get(0)); } @@ -190,6 +193,22 @@ public class ReloadingJVMCommandProcess { System.err.println("!!"); } + private static void isReloadableTypeCheck(String classname) { + try { + Class clazz = Class.forName(classname); + TypeRegistry tr = TypeRegistry.getTypeRegistryFor(clazz.getClassLoader()); + if (tr.getReloadableType(clazz) != null) { + System.out.println(classname + " is reloadable type"); + } + else { + System.out.println(classname + " is not reloadable type"); + } + } + catch (Exception e) { + e.printStackTrace(System.out); + } + } + private static void reloadCommand(String classname, String data) { try { Class clazz = Class.forName(classname); diff --git a/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTests.java b/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTests.java index fcceb75..44e806b 100644 --- a/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTests.java +++ b/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTests.java @@ -57,6 +57,7 @@ import org.objectweb.asm.tree.LocalVariableNode; import org.objectweb.asm.tree.MethodNode; import org.springsource.loaded.ClassRenamer; import org.springsource.loaded.Constants; +import org.springsource.loaded.GlobalConfiguration; import org.springsource.loaded.ISMgr; import org.springsource.loaded.MethodMember; import org.springsource.loaded.NameRegistry; @@ -1009,6 +1010,8 @@ public abstract class SpringLoadedTests implements Constants { protected TypeRegistry getTypeRegistry(String includePatterns) { TypeRegistry.reinitialize(); TypeRegistry tr = TypeRegistry.getTypeRegistryFor(binLoader); + GlobalConfiguration.InTestMode = true; + GlobalConfiguration.allowSplitPackages = true; Properties p = new Properties(); if (includePatterns != null) { p.setProperty(TypeRegistry.Key_Inclusions, includePatterns); diff --git a/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTestsInSeparateJVM.java b/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTestsInSeparateJVM.java index 2a5f6ae..89b46e8 100644 --- a/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTestsInSeparateJVM.java +++ b/springloaded/src/test/java/org/springsource/loaded/test/SpringLoadedTestsInSeparateJVM.java @@ -17,6 +17,7 @@ package org.springsource.loaded.test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import org.junit.After; import org.junit.AfterClass; @@ -376,6 +377,43 @@ public class SpringLoadedTestsInSeparateJVM extends SpringLoadedTests { assertStdoutContains("TopB.foo() running\nControllerB.foo() running again!\n", jvm.call("a", "foo")); } + /** + * GRAILS-9061. + * + * To build the jar see testdata/jars/grailsplugins + */ + @Test + public void splitPackages() throws Exception { + String supertypeInJar = "grails.plugins.A"; // from the grailsPlugins.jar + String subtypeNotInJar = "grails.plugins.B"; + + jvm.shutdown(); + jvm = ReloadingJVM.launch("inclusions=grails.plugins..*"); + String path = jvm.copyJarToTestdataDirectory("grailsplugins/grailsPlugins.jar", "grailsplugins.jar"); + jvm.extendCp(path); + jvm.copyToTestdataDirectory(subtypeNotInJar); + + JVMOutput jo = null; + // Load the one from the jar + jvm.newInstance("a", supertypeInJar); + jo = jvm.call("a", "foo"); + assertContains("A.foo() running", jo.stdout); + + // Load the one not from the jar, which extends the jar one + jo = jvm.newInstance("b", subtypeNotInJar); + jo = jvm.call("b", "foo"); + System.out.println(jo); + assertContains("B.foo() running", jo.stdout); + + assertTrue(jvm.isReloadableType(supertypeInJar)); + assertTrue(jvm.isReloadableType(subtypeNotInJar)); + + jvm.updateClass(subtypeNotInJar, retrieveRename(subtypeNotInJar, subtypeNotInJar + "2")); + waitForReloadToOccur(); + jo = jvm.call("b", "foo"); + assertContains("B2.foo() running", jo.stdout); + } + // --- private void waitForReloadToOccur() { diff --git a/testdata/jars/grailsplugins/A.class b/testdata/jars/grailsplugins/A.class new file mode 100644 index 0000000000000000000000000000000000000000..d822b42ba1a86818d4455d5e3baf795765e1f328 GIT binary patch literal 258 zcmZ8bONzok6s#BX8Dsq54Q8h01qX;7FV;W zdR4`v>i)Ri0L(B5;1C9BDYdD@$uz0XE3pjVvYW|2DU8fh@e^k%sR{m<&UL*Yv_|6{ z!Ce)56{3Z91P?x;ztOq+ZjP}kw=y=0Fx(W0G&@;ptKX(uA9Q82|GjfU_**n(qSo4e w`^%{vflqKH4+b;YD#n17cxk|!T<~#b!$HVsWAH z?*wvmqK}~HP!&*Sl#9o2C_iViN72flh9!#r>;dIvsu))0&8Ha*a9>cVHD0l?0C*%> QOqr%EAnqKpM`{lH-?xxnxc~qF literal 0 HcmV?d00001 diff --git a/testdata/jars/grailsplugins/grailsplugins.jar b/testdata/jars/grailsplugins/grailsplugins.jar new file mode 100644 index 0000000000000000000000000000000000000000..693db966692fc34a4c129067aeed9b5552cc5104 GIT binary patch literal 486 zcmWIWW@Zs#;Nak3(2RQL#()IG7+4t6ixM+)iuDU}O4Bp*iuE1!l5-M^i~pTsV8{(P z?RVHfK<4{I6U7Ucg6^GaM|*5hmC7-VQrFPHZsAwmWy0;W_8k zs?U