Defensively handle fast class generation failure for individual methods

Includes rethrowing of last actual defineClass exception encountered.

Closes gh-27490

spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java

@@ -679,14 +679,20 @@ class CglibAopProxy implements AopProxy, Serializable {
 				Object retVal;
 				// Check whether we only have one InvokerInterceptor: that is,
 				// no real advice, but just reflective invocation of the target.
-				if (chain.isEmpty() && Modifier.isPublic(method.getModifiers())) {
+				if (chain.isEmpty() && CglibMethodInvocation.isMethodProxyCompatible(method)) {
 					// We can skip creating a MethodInvocation: just invoke the target directly.
 					// Note that the final invoker must be an InvokerInterceptor, so we know
 					// it does nothing but a reflective operation on the target, and no hot
 					// swapping or fancy proxying.
 					Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args);
+					try {
 						retVal = methodProxy.invoke(target, argsToUse);
 					}
+					catch (CodeGenerationException ex) {
+						CglibMethodInvocation.logFastClassGenerationFailure(method);
+						retVal = AopUtils.invokeJoinpointUsingReflection(target, method, argsToUse);
+					}
+				}
 				else {
 					// We need to create a method invocation...
 					retVal = new CglibMethodInvocation(proxy, target, method, args, targetClass, chain, methodProxy).proceed();
@@ -737,10 +743,7 @@ class CglibAopProxy implements AopProxy, Serializable {
 			super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers);
 
 			// Only use method proxy for public methods not derived from java.lang.Object
-			this.methodProxy = (Modifier.isPublic(method.getModifiers()) &&
+			this.methodProxy = (isMethodProxyCompatible(method) ? methodProxy : null);
-					method.getDeclaringClass() != Object.class && !AopUtils.isEqualsMethod(method) &&
-					!AopUtils.isHashCodeMethod(method) && !AopUtils.isToStringMethod(method) ?
-					methodProxy : null);
 		}
 
 		@Override
@@ -776,11 +779,26 @@ class CglibAopProxy implements AopProxy, Serializable {
 		@Override
 		protected Object invokeJoinpoint() throws Throwable {
 			if (this.methodProxy != null) {
+				try {
 					return this.methodProxy.invoke(this.target, this.arguments);
 				}
-			else {
+				catch (CodeGenerationException ex) {
+					logFastClassGenerationFailure(this.method);
+				}
+			}
 			return super.invokeJoinpoint();
 		}
+
+		static boolean isMethodProxyCompatible(Method method) {
+			return (Modifier.isPublic(method.getModifiers()) &&
+					method.getDeclaringClass() != Object.class && !AopUtils.isEqualsMethod(method) &&
+					!AopUtils.isHashCodeMethod(method) && !AopUtils.isToStringMethod(method));
+		}
+
+		static void logFastClassGenerationFailure(Method method) {
+			if (logger.isDebugEnabled()) {
+				logger.debug("Failed to generate CGLIB fast class for method: " + method);
+			}
 		}
 	}
 

spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java

@@ -63,17 +63,16 @@ public class ReflectUtils {
 
 	private static final Method classLoaderDefineClassMethod;
 
-	private static final ProtectionDomain PROTECTION_DOMAIN;
-
 	private static final Throwable THROWABLE;
 
+	private static final ProtectionDomain PROTECTION_DOMAIN;
+
 	private static final List<Method> OBJECT_METHODS = new ArrayList<Method>();
 
 	static {
 		Method privateLookupIn;
 		Method lookupDefineClass;
 		Method classLoaderDefineClass;
-		ProtectionDomain protectionDomain;
 		Throwable throwable = null;
 		try {
 			privateLookupIn = (Method) AccessController.doPrivileged(new PrivilegedExceptionAction() {
@@ -102,9 +101,22 @@ public class ReflectUtils {
 							String.class, byte[].class, Integer.TYPE, Integer.TYPE, ProtectionDomain.class);
 				}
 			});
-			protectionDomain = getProtectionDomain(ReflectUtils.class);
+		}
-			AccessController.doPrivileged(new PrivilegedExceptionAction() {
+		catch (Throwable t) {
-				public Object run() throws Exception {
+			privateLookupIn = null;
+			lookupDefineClass = null;
+			classLoaderDefineClass = null;
+			throwable = t;
+		}
+
+		privateLookupInMethod = privateLookupIn;
+		lookupDefineClassMethod = lookupDefineClass;
+		classLoaderDefineClassMethod = classLoaderDefineClass;
+		THROWABLE = throwable;
+		PROTECTION_DOMAIN = getProtectionDomain(ReflectUtils.class);
+
+		AccessController.doPrivileged(new PrivilegedAction() {
+			public Object run() {
 				Method[] methods = Object.class.getDeclaredMethods();
 				for (Method method : methods) {
 					if ("finalize".equals(method.getName())
@@ -117,24 +129,9 @@ public class ReflectUtils {
 			}
 		});
 	}
-		catch (Throwable t) {
-			privateLookupIn = null;
-			lookupDefineClass = null;
-			classLoaderDefineClass = null;
-			protectionDomain = null;
-			throwable = t;
-		}
-		privateLookupInMethod = privateLookupIn;
-		lookupDefineClassMethod = lookupDefineClass;
-		classLoaderDefineClassMethod = classLoaderDefineClass;
-		PROTECTION_DOMAIN = protectionDomain;
-		THROWABLE = throwable;
-	}
 	// SPRING PATCH END
 
-	private static final String[] CGLIB_PACKAGES = {
+	private static final String[] CGLIB_PACKAGES = {"java.lang"};
-			"java.lang",
-	};
 
 	static {
 		primitives.put("byte", Byte.TYPE);
@@ -499,6 +496,7 @@ public class ReflectUtils {
 			ProtectionDomain protectionDomain, Class<?> contextClass) throws Exception {
 
 		Class c = null;
+		Throwable t = THROWABLE;
 
 		// Preferred option: JDK 9+ Lookup.defineClass API if ClassLoader matches
 		if (contextClass != null && contextClass.getClassLoader() == loader &&
@@ -516,6 +514,7 @@ public class ReflectUtils {
 				// in case of plain LinkageError (class already defined)
 				// or IllegalArgumentException (class in different package):
 				// fall through to traditional ClassLoader.defineClass below
+				t = ex;
 			}
 			catch (Throwable ex) {
 				throw new CodeGenerationException(ex);
@@ -539,9 +538,11 @@ public class ReflectUtils {
 					throw new CodeGenerationException(ex.getTargetException());
 				}
 				// in case of UnsupportedOperationException, fall through
+				t = ex.getTargetException();
 			}
 			catch (Throwable ex) {
 				// publicDefineClass method not available -> fall through
+				t = ex;
 			}
 
 			// Classic option: protected ClassLoader.defineClass method
@@ -562,6 +563,7 @@ public class ReflectUtils {
 					if (!ex.getClass().getName().endsWith("InaccessibleObjectException")) {
 						throw new CodeGenerationException(ex);
 					}
+					t = ex;
 				}
 			}
 		}
@@ -584,7 +586,7 @@ public class ReflectUtils {
 
 		// No defineClass variant available at all?
 		if (c == null) {
-			throw new CodeGenerationException(THROWABLE);
+			throw new CodeGenerationException(t);
 		}
 
 		// Force static initializers to run.