diff options
15 files changed, 376 insertions, 66 deletions
diff --git a/bundles/org.eclipse.osgi.tests/bundles_src/classloader.hooks.a/org/eclipse/osgi/tests/classloader/hooks/a/TestHookConfigurator.java b/bundles/org.eclipse.osgi.tests/bundles_src/classloader.hooks.a/org/eclipse/osgi/tests/classloader/hooks/a/TestHookConfigurator.java index 0259d4fd2..db458e144 100644 --- a/bundles/org.eclipse.osgi.tests/bundles_src/classloader.hooks.a/org/eclipse/osgi/tests/classloader/hooks/a/TestHookConfigurator.java +++ b/bundles/org.eclipse.osgi.tests/bundles_src/classloader.hooks.a/org/eclipse/osgi/tests/classloader/hooks/a/TestHookConfigurator.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2016 IBM Corporation and others. + * Copyright (c) 2013, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,6 +10,7 @@ *******************************************************************************/ package org.eclipse.osgi.tests.classloader.hooks.a; +import org.eclipse.osgi.container.Module; import org.eclipse.osgi.internal.hookregistry.*; import org.eclipse.osgi.internal.loader.classpath.ClasspathEntry; import org.eclipse.osgi.internal.loader.classpath.ClasspathManager; @@ -18,6 +19,13 @@ import org.eclipse.osgi.storage.bundlefile.BundleEntry; public class TestHookConfigurator implements HookConfigurator { private static final String REJECT_PROP = "classloader.hooks.a.reject"; private static final String BAD_TRANSFORM_PROP = "classloader.hooks.a.bad.transform"; + private static final String RECURSION_LOAD = "classloader.hooks.a.recursion.load"; + private static final String RECURSION_LOAD_SUPPORTED = "classloader.hooks.a.recursion.load.supported"; + final ThreadLocal<Boolean> doingRecursionLoad = new ThreadLocal<Boolean>() { + protected Boolean initialValue() { + return false; + }; + }; public void addHooks(HookRegistry hookRegistry) { hookRegistry.addClassLoaderHook(new ClassLoaderHook() { @@ -32,9 +40,34 @@ public class TestHookConfigurator implements HookConfigurator { if (Boolean.getBoolean(BAD_TRANSFORM_PROP)) { return new byte[] {'b', 'a', 'd', 'b', 'y', 't', 'e', 's'}; } + if (Boolean.getBoolean(RECURSION_LOAD)) { + if (isProcessClassRecursionSupported() && doingRecursionLoad.get()) { + return null; + } + Module m = manager.getGeneration().getBundleInfo().getStorage().getModuleContainer().getModule(1); + doingRecursionLoad.set(true); + try { + m.getCurrentRevision().getWiring().getClassLoader().loadClass("substitutes.x.Ax"); + if (!isProcessClassRecursionSupported()) { + throw new LinkageError("Recursion is no supported."); + } + } catch (ClassNotFoundException e) { + if (isProcessClassRecursionSupported()) { + throw new LinkageError("Recursion should be supported."); + } + // expected + } finally { + doingRecursionLoad.set(false); + } + } return null; } + @Override + public boolean isProcessClassRecursionSupported() { + return Boolean.getBoolean(RECURSION_LOAD_SUPPORTED); + } + }); } } diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java index 20fbe4366..1acbfb4a1 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java @@ -15,6 +15,7 @@ import java.lang.reflect.Method; import java.net.*; import java.util.*; import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicBoolean; import javax.jws.WebService; import javax.xml.namespace.QName; import javax.xml.ws.Endpoint; @@ -25,6 +26,8 @@ import org.eclipse.osgi.tests.OSGiTestsActivator; import org.osgi.framework.*; import org.osgi.framework.hooks.resolver.ResolverHook; import org.osgi.framework.hooks.resolver.ResolverHookFactory; +import org.osgi.framework.hooks.weaving.WeavingHook; +import org.osgi.framework.hooks.weaving.WovenClass; import org.osgi.framework.namespace.PackageNamespace; import org.osgi.framework.wiring.*; import org.osgi.service.packageadmin.ExportedPackage; @@ -2221,6 +2224,84 @@ public class ClassLoadingBundleTests extends AbstractBundleTests { } } + public void testRecursiveWeavingHookFactory() { + final ThreadLocal<Boolean> testThread = new ThreadLocal<Boolean>() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; + + testThread.set(Boolean.TRUE); + final Set<String> weavingHookClasses = new HashSet<String>(); + final List<WovenClass> called = new ArrayList<WovenClass>(); + final AtomicBoolean loadNewClassInWeave = new AtomicBoolean(false); + + ServiceFactory<WeavingHook> topFactory = new ServiceFactory<WeavingHook>() { + @Override + public WeavingHook getService(Bundle bundle, ServiceRegistration<WeavingHook> registration) { + if (!testThread.get()) { + return null; + } + WeavingHook hook = new WeavingHook() { + + @Override + public void weave(WovenClass wovenClass) { + if (loadNewClassInWeave.get()) { + // Force a load of inner class + Runnable run = new Runnable() { + @Override + public void run() { + // nothing + } + }; + run.run(); + weavingHookClasses.add(run.getClass().getName()); + } + called.add(wovenClass); + } + }; + weavingHookClasses.add(hook.getClass().getName()); + return hook; + } + + @Override + public void ungetService(Bundle bundle, ServiceRegistration<WeavingHook> registration, WeavingHook service) { + // nothing + } + }; + ServiceRegistration<WeavingHook> reg = getContext().registerService(WeavingHook.class, topFactory, null); + + Runnable run = null; + try { + // force call to factory without protection of the framework recursion checks + topFactory.getService(null, null); + + // set flag to load inner class while weaving + loadNewClassInWeave.set(true); + + // Force a load of inner class + run = new Runnable() { + @Override + public void run() { + // nothing + } + }; + run.run(); + } finally { + reg.unregister(); + } + + assertEquals("Unexpected number of woven classes.", 3, called.size()); + for (WovenClass wovenClass : called) { + if (weavingHookClasses.contains(wovenClass.getClassName())) { + assertNull("Did not expect to find class: " + wovenClass.getDefinedClass(), wovenClass.getDefinedClass()); + } else { + assertEquals("Expected the inner runnable class.", run.getClass(), wovenClass.getDefinedClass()); + } + } + } + public void testLoaderUninstalledBundle() throws BundleException, IOException { String testResourcePath = "testResource"; File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$ diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/ClassLoaderHookTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/ClassLoaderHookTests.java index 56d2b76e1..d342e8ecd 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/ClassLoaderHookTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/ClassLoaderHookTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2016 IBM Corporation and others. + * Copyright (c) 2013, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -32,6 +32,8 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { private static final String HOOK_CONFIGURATOR_CLASS = "org.eclipse.osgi.tests.classloader.hooks.a.TestHookConfigurator"; private static final String REJECT_PROP = "classloader.hooks.a.reject"; private static final String BAD_TRANSFORM_PROP = "classloader.hooks.a.bad.transform"; + private static final String RECURSION_LOAD = "classloader.hooks.a.recursion.load"; + private static final String RECURSION_LOAD_SUPPORTED = "classloader.hooks.a.recursion.load.supported"; private Map<String, String> configuration; private Framework framework; @@ -41,6 +43,8 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { super.setUp(); setRejectTransformation(false); setBadTransform(false); + setRecursionLoad(false); + setRecursionLoadSupported(false); String loc = bundleInstaller.getBundleLocation(HOOK_CONFIGURATOR_BUNDLE); loc = loc.substring(loc.indexOf("file:")); classLoader.addURL(new URL(loc)); @@ -65,7 +69,7 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { return framework.getBundleContext().installBundle(location); } - private void setRejectTransformation(boolean value) throws Exception { + private void setRejectTransformation(boolean value) { System.setProperty(REJECT_PROP, Boolean.toString(value)); } @@ -73,6 +77,14 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { System.setProperty(BAD_TRANSFORM_PROP, Boolean.toString(value)); } + private void setRecursionLoad(boolean value) { + System.setProperty(RECURSION_LOAD, Boolean.toString(value)); + } + + private void setRecursionLoadSupported(boolean value) { + System.setProperty(RECURSION_LOAD_SUPPORTED, Boolean.toString(value)); + } + public void testRejectTransformationFromWeavingHook() throws Exception { setRejectTransformation(true); initAndStartFramework(); @@ -95,6 +107,7 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { refreshBundles(Collections.singleton(b)); try { b.loadClass(TEST_CLASSNAME); + fail("Expected a ClassFormatError."); } catch (ClassFormatError e) { // expected } @@ -115,11 +128,27 @@ public class ClassLoaderHookTests extends AbstractFrameworkHookTests { refreshBundles(Collections.singleton(b)); try { b.loadClass(TEST_CLASSNAME); + fail("Expected a ClassFormatError."); } catch (ClassFormatError e) { // expected } } + public void testRecursionFromClassLoadingHookNotSupported() throws Exception { + setRecursionLoad(true); + initAndStartFramework(); + Bundle b = installBundle(); + b.loadClass(TEST_CLASSNAME); + } + + public void testRecursionFromClassLoadingHookIsSupported() throws Exception { + setRecursionLoad(true); + setRecursionLoadSupported(true); + initAndStartFramework(); + Bundle b = installBundle(); + b.loadClass(TEST_CLASSNAME); + } + private void refreshBundles(Collection<Bundle> bundles) throws InterruptedException { final CountDownLatch refreshSignal = new CountDownLatch(1); framework.adapt(FrameworkWiring.class).refreshBundles(bundles, new FrameworkListener() { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java index a65bbe426..a19273b9e 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2003, 2016 IBM Corporation and others. + * Copyright (c) 2003, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -235,6 +235,11 @@ public class BundleContextImpl implements BundleContext, EventDispatcher<Object, public String getHookMethodName() { return "find"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxContainer.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxContainer.java index db901c525..b81704cc6 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxContainer.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxContainer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -19,6 +19,7 @@ import org.eclipse.osgi.framework.log.FrameworkLogEntry; import org.eclipse.osgi.framework.util.SecureAction; import org.eclipse.osgi.internal.framework.legacy.PackageAdminImpl; import org.eclipse.osgi.internal.framework.legacy.StartLevelImpl; +import org.eclipse.osgi.internal.hookregistry.ClassLoaderHook; import org.eclipse.osgi.internal.hookregistry.HookRegistry; import org.eclipse.osgi.internal.location.EquinoxLocations; import org.eclipse.osgi.internal.log.EquinoxLogServices; @@ -46,6 +47,7 @@ public class EquinoxContainer implements ThreadFactory, Runnable { private final Set<String> bootDelegation; private final String[] bootDelegationStems; private final boolean bootDelegateAll; + private final boolean isProcessClassRecursionSupportedByAll; private final EquinoxEventPublisher eventPublisher; private final Object monitor = new Object(); @@ -97,6 +99,13 @@ public class EquinoxContainer implements ThreadFactory, Runnable { bootDelegateAll = delegateAllValue; bootDelegation = exactMatch; bootDelegationStems = stemMatch.isEmpty() ? null : stemMatch.toArray(new String[stemMatch.size()]); + + // Detect if all hooks can support recursive class processing + boolean supportRecursion = true; + for (ClassLoaderHook hook : equinoxConfig.getHookRegistry().getClassLoaderHooks()) { + supportRecursion &= hook.isProcessClassRecursionSupported(); + } + isProcessClassRecursionSupportedByAll = supportRecursion; } public Storage getStorage() { @@ -143,6 +152,10 @@ public class EquinoxContainer implements ThreadFactory, Runnable { return false; } + public boolean isProcessClassRecursionSupportedByAll() { + return isProcessClassRecursionSupportedByAll; + } + void init() { eventPublisher.init(); synchronized (this.monitor) { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxEventPublisher.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxEventPublisher.java index 232c8de0c..9884a785a 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxEventPublisher.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxEventPublisher.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -247,6 +247,11 @@ public class EquinoxEventPublisher { public String getHookMethodName() { return "event"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/OSGiFrameworkHooks.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/OSGiFrameworkHooks.java index 464e9dc20..14a6df5da 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/OSGiFrameworkHooks.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/OSGiFrameworkHooks.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -115,6 +115,11 @@ class OSGiFrameworkHooks { public String getHookMethodName() { return "filterCollisions"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hookregistry/ClassLoaderHook.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hookregistry/ClassLoaderHook.java index f921acce7..0f0b02b92 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hookregistry/ClassLoaderHook.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hookregistry/ClassLoaderHook.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2013 IBM Corporation and others. + * Copyright (c) 2005, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -292,4 +292,17 @@ public abstract class ClassLoaderHook { return null; } + /** + * Returns true if this hook can support invoking + * {@link ClassLoaderHook#processClass(String, byte[], ClasspathEntry, BundleEntry, ClasspathManager) processClass} + * recursively for the same class name. If false is returned then a class + * loading error will occur if recursive class processing is detected. + * <p> + * This method must return a constant boolean value. + * @return true if recursing class processing is supported + */ + public boolean isProcessClassRecursionSupported() { + return false; + } + } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/DevClassLoadingHook.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/DevClassLoadingHook.java index d936f4ef6..99353e819 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/DevClassLoadingHook.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/DevClassLoadingHook.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2006, 2012 IBM Corporation and others. + * Copyright (c) 2006, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -109,4 +109,10 @@ public class DevClassLoadingHook extends ClassLoaderHook implements KeyedElement public int getKeyHashCode() { return HASHCODE; } + + @Override + public boolean isProcessClassRecursionSupported() { + return true; + } + } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/EclipseLazyStarter.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/EclipseLazyStarter.java index 7312f8fc0..26edbbac6 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/EclipseLazyStarter.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/hooks/EclipseLazyStarter.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2006, 2016 IBM Corporation and others. + * Copyright (c) 2006, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -174,4 +174,9 @@ public class EclipseLazyStarter extends ClassLoaderHook { return ((includes == null || includes.contains(packageName)) && (excludes == null || !excludes.contains(packageName))); } + @Override + public boolean isProcessClassRecursionSupported() { + return true; + } + } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java index 8139ded11..10ae89de1 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/classpath/ClasspathManager.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2016 IBM Corporation and others. + * Copyright (c) 2005, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -63,7 +63,7 @@ public class ClasspathManager { // a Map<String,String> where "libname" is the key and libpath" is the value private ArrayMap<String, String> loadedLibraries = null; // used to detect recusive defineClass calls for the same class on the same class loader (bug 345500) - private ThreadLocal<Collection<String>> currentlyDefining = new ThreadLocal<>(); + private ThreadLocal<DefineContext> currentDefineContext = new ThreadLocal<>(); /** * Constructs a classpath manager for the given generation and module class loader @@ -574,25 +574,20 @@ public class ClasspathManager { Debug.println(" defining class " + name); //$NON-NLS-1$ } - Collection<String> current = currentlyDefining.get(); - if (current == null) { - current = new ArrayList<>(5); - currentlyDefining.set(current); - } - if (current.contains(name)) - return null; // avoid recursive defines (bug 345500) try { - current.add(name); return defineClass(name, classbytes, classpathEntry, entry, hooks); } catch (Error e) { if (debug.DEBUG_LOADER) Debug.println(" error defining class " + name); //$NON-NLS-1$ throw e; - } finally { - current.remove(name); } } + static class DefineContext { + Collection<String> currentlyProcessing = new ArrayList<>(5); + Collection<String> currentlyDefining = new ArrayList<>(5); + } + /** * Defines the specified class. This method will first call all the configured class loader hooks * {@link ClassLoadingHook#processClass(String, byte[], ClasspathEntry, BundleEntry, ClasspathManager)} @@ -610,37 +605,90 @@ public class ClasspathManager { */ private Class<?> defineClass(String name, byte[] classbytes, ClasspathEntry classpathEntry, BundleEntry entry, List<ClassLoaderHook> hooks) { DefineClassResult result = null; + boolean recursionDetected = false; try { definePackage(name, classpathEntry); - for (ClassLoaderHook hook : hooks) { - byte[] modifiedBytes = hook.processClass(name, classbytes, classpathEntry, entry, this); - if (modifiedBytes != null) { - // the WeavingHookConfigurator already calls the rejectTransformation method; avoid calling it again. - if (!(hook instanceof WeavingHookConfigurator)) { - for (ClassLoaderHook rejectHook : hooks) { - if (rejectHook.rejectTransformation(name, modifiedBytes, classpathEntry, entry, this)) { - modifiedBytes = null; - break; - } + DefineContext context = currentDefineContext.get(); + if (context == null) { + context = new DefineContext(); + currentDefineContext.set(context); + } + + // First call the hooks that do not handle recursion themselves + if (!hookRegistry.getContainer().isProcessClassRecursionSupportedByAll()) { + // One or more hooks do not support recursive class processing. + // We need to detect recursions for this set of hooks. + if (context.currentlyProcessing.contains(name)) { + // Avoid recursion for the same class name for these hooks + recursionDetected = true; + // TODO consider thrown a ClassCircularityError here + return null; + } + context.currentlyProcessing.add(name); + try { + + for (ClassLoaderHook hook : hooks) { + if (!hook.isProcessClassRecursionSupported()) { + classbytes = processClass(hook, name, classbytes, classpathEntry, entry, this, hooks); } } - if (modifiedBytes != null) { - classbytes = modifiedBytes; - } + } finally { + context.currentlyProcessing.remove(name); } } - result = classloader.defineClass(name, classbytes, classpathEntry); - } finally { - // only pass the newly defined class to the hook - Class<?> defined = result != null && result.defined ? result.clazz : null; + + // Now call the hooks that do support recursion without the check. for (ClassLoaderHook hook : hooks) { - hook.recordClassDefine(name, defined, classbytes, classpathEntry, entry, this); + if (hook.isProcessClassRecursionSupported()) { + // Note if the hooks don't take protective measures for a recursive class load here + // it will result in a stack overflow. + classbytes = processClass(hook, name, classbytes, classpathEntry, entry, this, hooks); + } + } + + if (context.currentlyDefining.contains(name)) { + // TODO consider thrown a ClassCircularityError here + return null; // avoid recursive defines (bug 345500) + } + context.currentlyDefining.add(name); + try { + result = classloader.defineClass(name, classbytes, classpathEntry); + } finally { + context.currentlyDefining.remove(name); + } + } finally { + // only call hooks if we properly called processClass above + if (!recursionDetected) { + // only pass the newly defined class to the hook + Class<?> defined = result != null && result.defined ? result.clazz : null; + for (ClassLoaderHook hook : hooks) { + hook.recordClassDefine(name, defined, classbytes, classpathEntry, entry, this); + } } } // return either the pre-loaded class or the newly defined class return result == null ? null : result.clazz; } + private byte[] processClass(ClassLoaderHook hook, String name, byte[] classbytes, ClasspathEntry classpathEntry, BundleEntry entry, ClasspathManager classpathManager, List<ClassLoaderHook> hooks) { + byte[] modifiedBytes = hook.processClass(name, classbytes, classpathEntry, entry, this); + if (modifiedBytes != null) { + // the WeavingHookConfigurator already calls the rejectTransformation method; avoid calling it again. + if (!(hook instanceof WeavingHookConfigurator)) { + for (ClassLoaderHook rejectHook : hooks) { + if (rejectHook.rejectTransformation(name, modifiedBytes, classpathEntry, entry, this)) { + modifiedBytes = null; + break; + } + } + } + if (modifiedBytes != null) { + classbytes = modifiedBytes; + } + } + return classbytes; + } + private void definePackage(String name, ClasspathEntry classpathEntry) { // Define the package if it is not the default package. int lastIndex = name.lastIndexOf('.'); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/HookContext.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/HookContext.java index 521917d1b..3bb5528a4 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/HookContext.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/HookContext.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2010, 2012 IBM Corporation and others. + * Copyright (c) 2010, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -46,4 +46,11 @@ public interface HookContext { * @return The hook method name called by this hook context. */ public String getHookMethodName(); + + /** + * Returns true if the given registration should be skipped. + * @param hookRegistration the registration to check + * @return true if the given registration should be skipped. + */ + public boolean skipRegistration(ServiceRegistration<?> hookRegistration); } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java index cc24fe06c..c3ac4d985 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2004, 2016 IBM Corporation and others. + * Copyright (c) 2004, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -1187,6 +1187,11 @@ public class ServiceRegistry { public String getHookMethodName() { return "find"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } @@ -1217,6 +1222,11 @@ public class ServiceRegistry { public String getHookMethodName() { return "event"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } @@ -1246,6 +1256,11 @@ public class ServiceRegistry { public String getHookMethodName() { return "event"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } @@ -1272,6 +1287,9 @@ public class ServiceRegistry { * @param hookContext Context to use when calling the hook service. */ private void notifyHookPrivileged(BundleContextImpl context, ServiceRegistrationImpl<?> registration, HookContext hookContext) { + if (hookContext.skipRegistration(registration)) { + return; + } Object hook = registration.getSafeService(context, ServiceConsumer.singletonConsumer); if (hook == null) { // if the hook is null return; @@ -1342,6 +1360,11 @@ public class ServiceRegistry { public String getHookMethodName() { return "added"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } @@ -1393,6 +1416,11 @@ public class ServiceRegistry { public String getHookMethodName() { return added ? "added" : "removed"; //$NON-NLS-1$ //$NON-NLS-2$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }); } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WeavingHookConfigurator.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WeavingHookConfigurator.java index e37e2670b..e1f830469 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WeavingHookConfigurator.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WeavingHookConfigurator.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2010, 2016 IBM Corporation and others. + * Copyright (c) 2010, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -22,10 +22,15 @@ import org.eclipse.osgi.storage.bundlefile.BundleEntry; import org.osgi.framework.*; public class WeavingHookConfigurator extends ClassLoaderHook { + static class WovenClassContext { + List<WovenClassImpl> wovenClassStack = new ArrayList<>(6); + List<String> processClassNameStack = new ArrayList<>(6); + } + // holds the map of black listed hooks. Use weak map to avoid pinning and simplify cleanup. private final Map<ServiceRegistration<?>, Boolean> blackList = Collections.synchronizedMap(new WeakHashMap<ServiceRegistration<?>, Boolean>()); // holds the stack of WovenClass objects currently being used to define classes - private final ThreadLocal<List<WovenClassImpl>> wovenClassStack = new ThreadLocal<>(); + private final ThreadLocal<WovenClassContext> wovenClassContext = new ThreadLocal<>(); private final EquinoxContainer container; @@ -45,34 +50,48 @@ public class WeavingHookConfigurator extends ClassLoaderHook { BundleLoader loader = classLoader.getBundleLoader(); // create a woven class object and add it to the thread local stack WovenClassImpl wovenClass = new WovenClassImpl(name, classbytes, entry, classpathEntry, loader, container, blackList); - List<WovenClassImpl> wovenClasses = wovenClassStack.get(); - if (wovenClasses == null) { - wovenClasses = new ArrayList<>(6); - wovenClassStack.set(wovenClasses); + WovenClassContext context = wovenClassContext.get(); + if (context == null) { + context = new WovenClassContext(); + wovenClassContext.set(context); } - wovenClasses.add(wovenClass); - // call the weaving hooks - try { - return wovenClass.callHooks(); - } catch (Throwable t) { - ServiceRegistration<?> errorHook = wovenClass.getErrorHook(); - Bundle errorBundle = errorHook != null ? errorHook.getReference().getBundle() : manager.getGeneration().getRevision().getBundle(); - container.getEventPublisher().publishFrameworkEvent(FrameworkEvent.ERROR, errorBundle, t); - // fail hard with a class loading error - ClassFormatError error = new ClassFormatError("Unexpected error from weaving hook."); //$NON-NLS-1$ - error.initCause(t); - throw error; + context.wovenClassStack.add(wovenClass); + // If we detect recursion for the same class name then we will not call the + // weaving hooks for the second request to load. + // Note that this means the actual bytes that get used to define the class + // will not have been woven at all. + if (!context.processClassNameStack.contains(name)) { + context.processClassNameStack.add(name); + // call the weaving hooks + try { + return wovenClass.callHooks(); + } catch (Throwable t) { + ServiceRegistration<?> errorHook = wovenClass.getErrorHook(); + Bundle errorBundle = errorHook != null ? errorHook.getReference().getBundle() : manager.getGeneration().getRevision().getBundle(); + container.getEventPublisher().publishFrameworkEvent(FrameworkEvent.ERROR, errorBundle, t); + // fail hard with a class loading error + ClassFormatError error = new ClassFormatError("Unexpected error from weaving hook."); //$NON-NLS-1$ + error.initCause(t); + throw error; + } finally { + context.processClassNameStack.remove(name); + } } + return null; } public void recordClassDefine(String name, Class<?> clazz, byte[] classbytes, ClasspathEntry classpathEntry, BundleEntry entry, ClasspathManager manager) { // here we assume the stack contans a woven class with the same name as the class we are defining. - List<WovenClassImpl> wovenClasses = wovenClassStack.get(); - if (wovenClasses == null || wovenClasses.size() == 0) + WovenClassContext context = wovenClassContext.get(); + if (context == null || context.wovenClassStack.size() == 0) return; - WovenClassImpl wovenClass = wovenClasses.remove(wovenClasses.size() - 1); + WovenClassImpl wovenClass = context.wovenClassStack.remove(context.wovenClassStack.size() - 1); // inform the woven class about the class that was defined. wovenClass.setWeavingCompleted(clazz); } + @Override + public boolean isProcessClassRecursionSupported() { + return true; + } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WovenClassImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WovenClassImpl.java index fbd98bc13..89609baf7 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WovenClassImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/weaving/WovenClassImpl.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2010, 2013 IBM Corporation and others. + * Copyright (c) 2010, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -147,8 +147,11 @@ public final class WovenClassImpl implements WovenClass, HookContext { if (error != null) return; // do not call any other hooks once an error has occurred. if (hook instanceof WeavingHook) { - if (blackList.containsKey(hookRegistration)) - return; // black listed hook + if (skipRegistration(hookRegistration)) { + // Note we double check blacklist here just + // in case another thread blacklisted since the first check + return; + } if ((hookFlags & FLAG_HOOKCALLED) == 0) { hookFlags |= FLAG_HOOKCALLED; // only do this check on the first weaving hook call @@ -171,6 +174,11 @@ public final class WovenClassImpl implements WovenClass, HookContext { } } + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return blackList.containsKey(hookRegistration); + } + private boolean validBytes(byte[] checkBytes) { if (checkBytes == null || checkBytes.length < 4) return false; @@ -215,6 +223,11 @@ public final class WovenClassImpl implements WovenClass, HookContext { public String getHookMethodName() { return "modified"; //$NON-NLS-1$ } + + @Override + public boolean skipRegistration(ServiceRegistration<?> hookRegistration) { + return false; + } }; if (System.getSecurityManager() == null) registry.notifyHooksPrivileged(context); |