You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by mg...@apache.org on 2021/08/12 07:06:17 UTC

[wicket] branch WICKET-6913-replace-cglib-with-bytebuddy updated: WICKET-6913 Convert Objenesis proxy generator to ByteBuddy

This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a commit to branch WICKET-6913-replace-cglib-with-bytebuddy
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/WICKET-6913-replace-cglib-with-bytebuddy by this push:
     new 43f040e  WICKET-6913 Convert Objenesis proxy generator to ByteBuddy
43f040e is described below

commit 43f040e1ffc2ea03a458cb74aca186e2692e63e8
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
AuthorDate: Thu Aug 12 09:51:43 2021 +0300

    WICKET-6913 Convert Objenesis proxy generator to ByteBuddy
---
 .../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 15c050c..204c05e 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()
 	{