This is an automated email from the ASF dual-hosted git repository. mgrigorov pushed a commit to branch WICKET-6913-replace-cglib-with-bytebuddy-wicket-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit a480bcd353e0706a96a52d4199cf69e5fdf03d0c Author: Martin Tzvetanov Grigorov <mgrigo...@apache.org> AuthorDate: Thu Aug 12 09:51:43 2021 +0300 WICKET-6913 Convert Objenesis proxy generator to ByteBuddy (cherry picked from commit 43f040e1ffc2ea03a458cb74aca186e2692e63e8) --- .../apache/wicket/proxy/LazyInitProxyFactory.java | 44 ++++--- ...tor.java => ObjenesisByteBuddyInterceptor.java} | 8 +- .../proxy/objenesis/ObjenesisProxyFactory.java | 29 ++--- .../proxy/objenesis/ObjenesisProxyReplacement.java | 2 +- wicket-jmx/pom.xml | 5 +- wicket-jmx/src/main/java/module-info.java | 2 +- .../java/org/apache/wicket/jmx/Initializer.java | 130 ++++++++++++++------- .../org/apache/wicket/jmx/wrapper/Application.java | 21 ---- .../wicket/jmx/wrapper/ApplicationSettings.java | 9 -- 9 files changed, 130 insertions(+), 120 deletions(-) diff --git a/wicket-ioc/src/main/java/org/apache/wicket/proxy/LazyInitProxyFactory.java b/wicket-ioc/src/main/java/org/apache/wicket/proxy/LazyInitProxyFactory.java index d478a25..8a24bc2 100644 --- a/wicket-ioc/src/main/java/org/apache/wicket/proxy/LazyInitProxyFactory.java +++ b/wicket-ioc/src/main/java/org/apache/wicket/proxy/LazyInitProxyFactory.java @@ -41,6 +41,7 @@ import org.apache.wicket.Application; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.core.util.lang.WicketObjects; import org.apache.wicket.model.IModel; +import org.apache.wicket.proxy.objenesis.ObjenesisProxyFactory; import org.apache.wicket.util.io.IClusterable; /** @@ -173,31 +174,40 @@ public class LazyInitProxyFactory } else if (IS_OBJENESIS_AVAILABLE && !hasNoArgConstructor(type)) { - return null; //ObjenesisProxyFactory.createProxy(type, locator, WicketNamingPolicy.INSTANCE); + return ObjenesisProxyFactory.createProxy(type, locator); } else { - ClassLoader classLoader = resolveClassLoader(); - - Class<?> dynamicType = DYNAMIC_CLASS_CACHE.findOrInsert(classLoader, - new TypeCache.SimpleKey(type), - () -> BYTE_BUDDY - .subclass(type) - .implement(Serializable.class, ILazyInitProxy.class, IWriteReplace.class) - .method(ElementMatchers.any()) - .intercept(MethodDelegation.to(new ByteBuddyInterceptor(type, locator))) - .make() - .load(classLoader, ClassLoadingStrategy.Default.INJECTION) - .getLoaded()); - - try { - return dynamicType.getDeclaredConstructor().newInstance(); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + ByteBuddyInterceptor interceptor = new ByteBuddyInterceptor(type, locator); + Class<?> proxyClass = createProxyClass(type, interceptor); + + try + { + return proxyClass.getDeclaredConstructor().newInstance(); + } + catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) + { throw new WicketRuntimeException(e); } } } + public static Class<?> createProxyClass(Class<?> type, Object interceptor) + { + ClassLoader classLoader = resolveClassLoader(); + Class<?> dynamicType = DYNAMIC_CLASS_CACHE.findOrInsert(classLoader, + new TypeCache.SimpleKey(type), + () -> BYTE_BUDDY + .subclass(type) + .implement(Serializable.class, ILazyInitProxy.class, IWriteReplace.class) + .method(ElementMatchers.any()) + .intercept(MethodDelegation.to(interceptor)) + .make() + .load(classLoader, ClassLoadingStrategy.Default.INJECTION) + .getLoaded()); + return dynamicType; + } + private static ClassLoader resolveClassLoader() { ClassLoader classLoader = null; diff --git a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisCGLibInterceptor.java b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisByteBuddyInterceptor.java similarity index 82% rename from wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisCGLibInterceptor.java rename to wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisByteBuddyInterceptor.java index b4197ea..ee40a60 100644 --- a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisCGLibInterceptor.java +++ b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisByteBuddyInterceptor.java @@ -22,12 +22,12 @@ import org.apache.wicket.proxy.IProxyTargetLocator; import org.apache.wicket.proxy.LazyInitProxyFactory; /** - * Method interceptor for proxies representing concrete object not backed by an interface. These - * proxies are representing by cglib proxies. + * Method interceptor for proxies representing concrete object not backed by an interface. + * These proxies are representing by ByteBuddy proxies. */ -public class ObjenesisCGLibInterceptor extends LazyInitProxyFactory.AbstractByteBuddyInterceptor +public class ObjenesisByteBuddyInterceptor extends LazyInitProxyFactory.AbstractByteBuddyInterceptor { - public ObjenesisCGLibInterceptor(Class<?> type, IProxyTargetLocator locator) { + public ObjenesisByteBuddyInterceptor(Class<?> type, IProxyTargetLocator locator) { super(type, locator); } diff --git a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyFactory.java b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyFactory.java index 2be6129..050c255 100644 --- a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyFactory.java +++ b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyFactory.java @@ -16,13 +16,9 @@ */ package org.apache.wicket.proxy.objenesis; -import java.io.Serializable; - -import javax.security.auth.callback.Callback; - -import org.apache.wicket.proxy.ILazyInitProxy; +import net.bytebuddy.NamingStrategy; import org.apache.wicket.proxy.IProxyTargetLocator; -import org.apache.wicket.proxy.LazyInitProxyFactory.IWriteReplace; +import org.apache.wicket.proxy.LazyInitProxyFactory; import org.objenesis.ObjenesisStd; //import net.sf.cglib.core.NamingPolicy; @@ -34,23 +30,12 @@ public class ObjenesisProxyFactory { private static final ObjenesisStd OBJENESIS = new ObjenesisStd(false); - public static Object createProxy(final Class<?> type, final IProxyTargetLocator locator/*, NamingPolicy namingPolicy*/) + public static Object createProxy(final Class<?> type, final IProxyTargetLocator locator) { - ObjenesisCGLibInterceptor handler = new ObjenesisCGLibInterceptor(type, locator); + ObjenesisByteBuddyInterceptor interceptor = new ObjenesisByteBuddyInterceptor(type, locator); + final Class<?> proxyClass = LazyInitProxyFactory.createProxyClass(type, interceptor); -// Enhancer e = new Enhancer(); -// e.setInterfaces(new Class[]{Serializable.class, ILazyInitProxy.class, IWriteReplace.class}); -// e.setSuperclass(type); -// e.setCallbackType(handler.getClass()); -// e.setNamingPolicy(namingPolicy); -// Class<?> proxyClass = e.createClass(); -// -// Object instance = OBJENESIS.newInstance(proxyClass); -// -// // set callbacks directly (WICKET-6607) -// ((Factory) instance).setCallbacks(new Callback[]{handler}); -// -// return instance; - return null; + Object instance = OBJENESIS.newInstance(proxyClass); + return instance; } } diff --git a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyReplacement.java b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyReplacement.java index 7d2dc1d..f51258c 100644 --- a/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyReplacement.java +++ b/wicket-ioc/src/main/java/org/apache/wicket/proxy/objenesis/ObjenesisProxyReplacement.java @@ -50,6 +50,6 @@ class ObjenesisProxyReplacement implements IClusterable "] with the currently configured org.apache.wicket.application.IClassResolver"); throw new WicketRuntimeException(cause); } - return ObjenesisProxyFactory.createProxy(clazz, locator/*, LazyInitProxyFactory.WicketNamingPolicy.INSTANCE*/); + return ObjenesisProxyFactory.createProxy(clazz, locator); } } diff --git a/wicket-jmx/pom.xml b/wicket-jmx/pom.xml index c312f9b..b827a78 100644 --- a/wicket-jmx/pom.xml +++ b/wicket-jmx/pom.xml @@ -30,8 +30,9 @@ <dependencies> <dependency> - <groupId>cglib</groupId> - <artifactId>cglib</artifactId> + <groupId>net.bytebuddy</groupId> + <artifactId>byte-buddy</artifactId> + <version>1.11.12</version> </dependency> <dependency> <groupId>org.apache.wicket</groupId> diff --git a/wicket-jmx/src/main/java/module-info.java b/wicket-jmx/src/main/java/module-info.java index d07942d..a0366f8 100644 --- a/wicket-jmx/src/main/java/module-info.java +++ b/wicket-jmx/src/main/java/module-info.java @@ -20,7 +20,7 @@ module org.apache.wicket.jmx { requires org.apache.wicket.util; requires org.apache.wicket.core; requires org.slf4j; - requires cglib; + requires net.bytebuddy; provides org.apache.wicket.IInitializer with org.apache.wicket.jmx.Initializer; exports org.apache.wicket.jmx; diff --git a/wicket-jmx/src/main/java/org/apache/wicket/jmx/Initializer.java b/wicket-jmx/src/main/java/org/apache/wicket/jmx/Initializer.java index ca990ac..a2d9b3d 100644 --- a/wicket-jmx/src/main/java/org/apache/wicket/jmx/Initializer.java +++ b/wicket-jmx/src/main/java/org/apache/wicket/jmx/Initializer.java @@ -17,6 +17,7 @@ package org.apache.wicket.jmx; import java.lang.management.ManagementFactory; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -30,9 +31,16 @@ import javax.management.MalformedObjectNameException; import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; -import net.sf.cglib.core.DefaultNamingPolicy; -import net.sf.cglib.core.Predicate; -import net.sf.cglib.proxy.Enhancer; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.NamingStrategy; +import net.bytebuddy.TypeCache; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; +import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import net.bytebuddy.matcher.ElementMatchers; import org.apache.wicket.IInitializer; import org.apache.wicket.ThreadContext; @@ -68,7 +76,24 @@ import org.slf4j.LoggerFactory; */ public class Initializer implements IInitializer { - private static Logger log = LoggerFactory.getLogger(Initializer.class); + private static final Logger LOG = LoggerFactory.getLogger(Initializer.class); + + /** + * A cache used to store the dynamically generated classes by ByteBuddy. + * Without this cache a new class will be generated for each proxy creation + * and this will fill up the metaspace + */ + private static final TypeCache<TypeCache.SimpleKey> DYNAMIC_CLASS_CACHE = new TypeCache.WithInlineExpunction<>(TypeCache.Sort.SOFT); + + private static final ByteBuddy BYTE_BUDDY = new ByteBuddy() + .with(new NamingStrategy.AbstractBase() + { + @Override + protected String name(TypeDescription superClass) + { + return superClass.getName().replace(".wrapper", ""); + } + }); // It's best to store a reference to the MBeanServer rather than getting it // over and over @@ -90,7 +115,7 @@ public class Initializer implements IInitializer } catch (InstanceNotFoundException | MBeanRegistrationException e) { - log.error(e.getMessage(), e); + LOG.error(e.getMessage(), e); } } } @@ -110,7 +135,7 @@ public class Initializer implements IInitializer catch (SecurityException e) { // Ignore - we're not allowed to read this property. - log.warn("not allowed to read property wicket.mbean.server.agentid due to security settings; ignoring"); + LOG.warn("not allowed to read property wicket.mbean.server.agentid due to security settings; ignoring"); } if (agentId != null) { @@ -121,7 +146,7 @@ public class Initializer implements IInitializer } else { - log.error("unable to find mbean server with agent id " + agentId); + LOG.error("unable to find mbean server with agent id {}", agentId); } } if (mbeanServer == null) @@ -134,7 +159,7 @@ public class Initializer implements IInitializer catch (SecurityException e) { // Ignore - we're not allowed to read this property. - log.warn("not allowed to read property wicket.mbean.server.class due to security settings; ignoring"); + LOG.warn("not allowed to read property wicket.mbean.server.class due to security settings; ignoring"); } if (impl != null) { @@ -152,7 +177,7 @@ public class Initializer implements IInitializer } if (mbeanServer == null) { - log.error("unable to find mbean server of type '{}'", impl); + LOG.error("unable to find mbean server of type '{}'", impl); } } } @@ -162,7 +187,7 @@ public class Initializer implements IInitializer // never null } - log.info("registering Wicket mbeans with server '{}'", mbeanServer); + LOG.info("registering Wicket mbeans with server '{}'", mbeanServer); // register top level application object, but first check whether // multiple instances of the same application (name) are running and @@ -238,48 +263,67 @@ public class Initializer implements IInitializer registered.add(objectName); } - private Object createProxy(final org.apache.wicket.Application application, final Object o) + private static class Interceptor { - Enhancer e = new Enhancer(); - e.setInterfaces(o.getClass().getInterfaces()); - e.setSuperclass(Object.class); - e.setCallback(new net.sf.cglib.proxy.InvocationHandler() + private final org.apache.wicket.Application application; + private final Object object; + + private Interceptor(org.apache.wicket.Application application, Object object) { + this.application = application; + this.object = object; + } + + @RuntimeType + public Object intercept(final @Origin Method method, + final @AllArguments Object[] args) + throws Throwable { - @Override - public Object invoke(Object proxy, Method method, Object[] args) throws Throwable + boolean existed = ThreadContext.exists(); + + if (existed == false) { - boolean existed = ThreadContext.exists(); + ThreadContext.setApplication(application); + } + try + { + return method.invoke(object, args); + } + finally + { if (existed == false) { - ThreadContext.setApplication(application); - } - - try - { - return method.invoke(o, args); + ThreadContext.detach(); } - finally - { - if (existed == false) - { - ThreadContext.detach(); - } - } - } - }); - e.setNamingPolicy(new DefaultNamingPolicy() - { - @Override - public String getClassName(final String prefix, final String source, final Object key, - final Predicate names) - { - return o.getClass().getName().replace(".wrapper", ""); } - }); - e.setClassLoader(resolveClassLoader()); + } + } + + private Object createProxy(final org.apache.wicket.Application application, final Object o) + { + Class<?> type = o.getClass(); + ClassLoader classLoader = resolveClassLoader(); + + Class<?> proxyClass = DYNAMIC_CLASS_CACHE.findOrInsert(classLoader, + new TypeCache.SimpleKey(type), + () -> BYTE_BUDDY + .subclass(type) + .implement(type.getInterfaces()) + .method(ElementMatchers.any()) + .intercept(MethodDelegation.to(new Interceptor(application, o))) + .make() + .load(classLoader, ClassLoadingStrategy.Default.INJECTION) + .getLoaded()); - return e.create(); + + try + { + return proxyClass.getDeclaredConstructor(org.apache.wicket.Application.class).newInstance(application); + } + catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) + { + throw new WicketRuntimeException(e); + } } private static ClassLoader resolveClassLoader() diff --git a/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/Application.java b/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/Application.java index 56e0eff..3d23047 100644 --- a/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/Application.java +++ b/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/Application.java @@ -39,63 +39,42 @@ public class Application implements ApplicationMBean this.application = application; } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#clearMarkupCache() - */ @Override public void clearMarkupCache() throws IOException { application.getMarkupSettings().getMarkupFactory().getMarkupCache().clear(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#getApplicationClass() - */ @Override public String getApplicationClass() throws IOException { return application.getClass().getName(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#getConfigurationType() - */ @Override public String getConfigurationType() { return application.getConfigurationType().name(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#getHomePageClass() - */ @Override public String getHomePageClass() throws IOException { return application.getHomePage().getName(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#getMarkupCacheSize() - */ @Override public int getMarkupCacheSize() throws IOException { return application.getMarkupSettings().getMarkupFactory().getMarkupCache().size(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#getWicketVersion() - */ @Override public String getWicketVersion() throws IOException { return application.getFrameworkSettings().getVersion(); } - /** - * @see org.apache.wicket.jmx.ApplicationMBean#clearLocalizerCache() - */ @Override public void clearLocalizerCache() throws IOException { diff --git a/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/ApplicationSettings.java b/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/ApplicationSettings.java index a6a0d4c..c5bda35 100644 --- a/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/ApplicationSettings.java +++ b/wicket-jmx/src/main/java/org/apache/wicket/jmx/wrapper/ApplicationSettings.java @@ -40,18 +40,12 @@ public class ApplicationSettings implements ApplicationSettingsMBean this.application = application; } - /** - * @see org.apache.wicket.jmx.ApplicationSettingsMBean#getAccessDeniedPage() - */ @Override public String getAccessDeniedPage() { return Classes.name(application.getApplicationSettings().getAccessDeniedPage()); } - /** - * @see org.apache.wicket.jmx.ApplicationSettingsMBean#getClassResolver() - */ @Override public String getClassResolver() { @@ -64,9 +58,6 @@ public class ApplicationSettings implements ApplicationSettingsMBean return application.getApplicationSettings().getDefaultMaximumUploadSize().toString(); } - /** - * @see org.apache.wicket.jmx.ApplicationSettingsMBean#getInternalErrorPage() - */ @Override public String getInternalErrorPage() {