Skip to content

Commit a295a28

Browse files
committed
Defensively handle fast class generation failure for individual methods
Includes rethrowing of last actual defineClass exception encountered. Closes gh-27490
1 parent bfa01b3 commit a295a28

File tree

2 files changed

+52
-32
lines changed

2 files changed

+52
-32
lines changed

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

+27-9
Original file line numberDiff line numberDiff line change
@@ -679,13 +679,19 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
679679
Object retVal;
680680
// Check whether we only have one InvokerInterceptor: that is,
681681
// no real advice, but just reflective invocation of the target.
682-
if (chain.isEmpty() && Modifier.isPublic(method.getModifiers())) {
682+
if (chain.isEmpty() && CglibMethodInvocation.isMethodProxyCompatible(method)) {
683683
// We can skip creating a MethodInvocation: just invoke the target directly.
684684
// Note that the final invoker must be an InvokerInterceptor, so we know
685685
// it does nothing but a reflective operation on the target, and no hot
686686
// swapping or fancy proxying.
687687
Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args);
688-
retVal = methodProxy.invoke(target, argsToUse);
688+
try {
689+
retVal = methodProxy.invoke(target, argsToUse);
690+
}
691+
catch (CodeGenerationException ex) {
692+
CglibMethodInvocation.logFastClassGenerationFailure(method);
693+
retVal = AopUtils.invokeJoinpointUsingReflection(target, method, argsToUse);
694+
}
689695
}
690696
else {
691697
// We need to create a method invocation...
@@ -737,10 +743,7 @@ public CglibMethodInvocation(Object proxy, @Nullable Object target, Method metho
737743
super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers);
738744

739745
// Only use method proxy for public methods not derived from java.lang.Object
740-
this.methodProxy = (Modifier.isPublic(method.getModifiers()) &&
741-
method.getDeclaringClass() != Object.class && !AopUtils.isEqualsMethod(method) &&
742-
!AopUtils.isHashCodeMethod(method) && !AopUtils.isToStringMethod(method) ?
743-
methodProxy : null);
746+
this.methodProxy = (isMethodProxyCompatible(method) ? methodProxy : null);
744747
}
745748

746749
@Override
@@ -776,10 +779,25 @@ public Object proceed() throws Throwable {
776779
@Override
777780
protected Object invokeJoinpoint() throws Throwable {
778781
if (this.methodProxy != null) {
779-
return this.methodProxy.invoke(this.target, this.arguments);
782+
try {
783+
return this.methodProxy.invoke(this.target, this.arguments);
784+
}
785+
catch (CodeGenerationException ex) {
786+
logFastClassGenerationFailure(this.method);
787+
}
780788
}
781-
else {
782-
return super.invokeJoinpoint();
789+
return super.invokeJoinpoint();
790+
}
791+
792+
static boolean isMethodProxyCompatible(Method method) {
793+
return (Modifier.isPublic(method.getModifiers()) &&
794+
method.getDeclaringClass() != Object.class && !AopUtils.isEqualsMethod(method) &&
795+
!AopUtils.isHashCodeMethod(method) && !AopUtils.isToStringMethod(method));
796+
}
797+
798+
static void logFastClassGenerationFailure(Method method) {
799+
if (logger.isDebugEnabled()) {
800+
logger.debug("Failed to generate CGLIB fast class for method: " + method);
783801
}
784802
}
785803
}

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

+25-23
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,16 @@ private ReflectUtils() {
6363

6464
private static final Method classLoaderDefineClassMethod;
6565

66-
private static final ProtectionDomain PROTECTION_DOMAIN;
67-
6866
private static final Throwable THROWABLE;
6967

68+
private static final ProtectionDomain PROTECTION_DOMAIN;
69+
7070
private static final List<Method> OBJECT_METHODS = new ArrayList<Method>();
7171

7272
static {
7373
Method privateLookupIn;
7474
Method lookupDefineClass;
7575
Method classLoaderDefineClass;
76-
ProtectionDomain protectionDomain;
7776
Throwable throwable = null;
7877
try {
7978
privateLookupIn = (Method) AccessController.doPrivileged(new PrivilegedExceptionAction() {
@@ -102,39 +101,37 @@ public Object run() throws Exception {
102101
String.class, byte[].class, Integer.TYPE, Integer.TYPE, ProtectionDomain.class);
103102
}
104103
});
105-
protectionDomain = getProtectionDomain(ReflectUtils.class);
106-
AccessController.doPrivileged(new PrivilegedExceptionAction() {
107-
public Object run() throws Exception {
108-
Method[] methods = Object.class.getDeclaredMethods();
109-
for (Method method : methods) {
110-
if ("finalize".equals(method.getName())
111-
|| (method.getModifiers() & (Modifier.FINAL | Modifier.STATIC)) > 0) {
112-
continue;
113-
}
114-
OBJECT_METHODS.add(method);
115-
}
116-
return null;
117-
}
118-
});
119104
}
120105
catch (Throwable t) {
121106
privateLookupIn = null;
122107
lookupDefineClass = null;
123108
classLoaderDefineClass = null;
124-
protectionDomain = null;
125109
throwable = t;
126110
}
111+
127112
privateLookupInMethod = privateLookupIn;
128113
lookupDefineClassMethod = lookupDefineClass;
129114
classLoaderDefineClassMethod = classLoaderDefineClass;
130-
PROTECTION_DOMAIN = protectionDomain;
131115
THROWABLE = throwable;
116+
PROTECTION_DOMAIN = getProtectionDomain(ReflectUtils.class);
117+
118+
AccessController.doPrivileged(new PrivilegedAction() {
119+
public Object run() {
120+
Method[] methods = Object.class.getDeclaredMethods();
121+
for (Method method : methods) {
122+
if ("finalize".equals(method.getName())
123+
|| (method.getModifiers() & (Modifier.FINAL | Modifier.STATIC)) > 0) {
124+
continue;
125+
}
126+
OBJECT_METHODS.add(method);
127+
}
128+
return null;
129+
}
130+
});
132131
}
133132
// SPRING PATCH END
134133

135-
private static final String[] CGLIB_PACKAGES = {
136-
"java.lang",
137-
};
134+
private static final String[] CGLIB_PACKAGES = {"java.lang"};
138135

139136
static {
140137
primitives.put("byte", Byte.TYPE);
@@ -499,6 +496,7 @@ public static Class defineClass(String className, byte[] b, ClassLoader loader,
499496
ProtectionDomain protectionDomain, Class<?> contextClass) throws Exception {
500497

501498
Class c = null;
499+
Throwable t = THROWABLE;
502500

503501
// Preferred option: JDK 9+ Lookup.defineClass API if ClassLoader matches
504502
if (contextClass != null && contextClass.getClassLoader() == loader &&
@@ -516,6 +514,7 @@ public static Class defineClass(String className, byte[] b, ClassLoader loader,
516514
// in case of plain LinkageError (class already defined)
517515
// or IllegalArgumentException (class in different package):
518516
// fall through to traditional ClassLoader.defineClass below
517+
t = ex;
519518
}
520519
catch (Throwable ex) {
521520
throw new CodeGenerationException(ex);
@@ -539,9 +538,11 @@ public static Class defineClass(String className, byte[] b, ClassLoader loader,
539538
throw new CodeGenerationException(ex.getTargetException());
540539
}
541540
// in case of UnsupportedOperationException, fall through
541+
t = ex.getTargetException();
542542
}
543543
catch (Throwable ex) {
544544
// publicDefineClass method not available -> fall through
545+
t = ex;
545546
}
546547

547548
// Classic option: protected ClassLoader.defineClass method
@@ -562,6 +563,7 @@ public static Class defineClass(String className, byte[] b, ClassLoader loader,
562563
if (!ex.getClass().getName().endsWith("InaccessibleObjectException")) {
563564
throw new CodeGenerationException(ex);
564565
}
566+
t = ex;
565567
}
566568
}
567569
}
@@ -584,7 +586,7 @@ public static Class defineClass(String className, byte[] b, ClassLoader loader,
584586

585587
// No defineClass variant available at all?
586588
if (c == null) {
587-
throw new CodeGenerationException(THROWABLE);
589+
throw new CodeGenerationException(t);
588590
}
589591

590592
// Force static initializers to run.

0 commit comments

Comments
 (0)