Skip to content

Commit 5d8d1e5

Browse files
#1882 use ClassValue instead of synchronized WeakHashMap in ClassFactory (#1883)
* use ClassValue instead of synchronized WeakHashMap in ClassFactory (to avoid locking issues under heavy multithreaded load)
1 parent 121cc8e commit 5d8d1e5

File tree

1 file changed

+37
-45
lines changed

1 file changed

+37
-45
lines changed

jaxb-ri/core/src/main/java/org/glassfish/jaxb/core/v2/ClassFactory.java

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright (c) 1997, 2022 Oracle and/or its affiliates. All rights reserved.
3-
* Copyright (c) 2025 Contributors to the Eclipse Foundation. All rights reserved.
3+
* Copyright (c) 2025-2026 Contributors to the Eclipse Foundation. All rights reserved.
44
*
55
* This program and the accompanying materials are made available under the
66
* terms of the Eclipse Distribution License v. 1.0, which is available at
@@ -15,12 +15,8 @@
1515
import java.lang.reflect.InvocationTargetException;
1616
import java.lang.reflect.Method;
1717
import java.lang.reflect.Modifier;
18-
import java.lang.ref.WeakReference;
1918
import java.security.AccessController;
2019
import java.security.PrivilegedAction;
21-
import java.util.Collections;
22-
import java.util.Map;
23-
import java.util.WeakHashMap;
2420
import java.util.logging.Level;
2521
import java.util.logging.Logger;
2622

@@ -45,7 +41,34 @@ public final class ClassFactory {
4541
/**
4642
* Cache from a class to its default constructor.
4743
*/
48-
private static final Map<Class, WeakReference<Constructor>> constructorClassCache = Collections.synchronizedMap(new WeakHashMap<>());
44+
private static final ClassValue<Constructor> DECLARED_CTORS =
45+
new ClassValue<>() {
46+
@Override
47+
protected Constructor computeValue(Class<?> clazz) {
48+
Constructor cons;
49+
if (System.getSecurityManager() == null) {
50+
cons = tryGetDeclaredConstructor(clazz);
51+
} else {
52+
cons = AccessController.doPrivileged(
53+
(PrivilegedAction<Constructor<?>>) () -> tryGetDeclaredConstructor(clazz)
54+
);
55+
}
56+
57+
int classMod = clazz.getModifiers();
58+
59+
if(!Modifier.isPublic(classMod) || !Modifier.isPublic(cons.getModifiers())) {
60+
// attempt to make it work even if the constructor is not accessible
61+
try {
62+
cons.setAccessible(true);
63+
} catch(SecurityException e) {
64+
// but if we don't have a permission to do so, work gracefully.
65+
logger.log(Level.FINE, e, () -> "Unable to make the constructor of "+clazz+" accessible");
66+
throw e;
67+
}
68+
}
69+
return cons;
70+
}
71+
};
4972

5073
private ClassFactory() {}
5174

@@ -61,46 +84,15 @@ public static void cleanCache() {
6184
* Creates a new instance of the class but throw exceptions without catching it.
6285
*/
6386
public static <T> T create0( final Class<T> clazz ) throws IllegalAccessException, InvocationTargetException, InstantiationException {
64-
Constructor<T> cons = null;
65-
WeakReference<Constructor> consRef = constructorClassCache.get(clazz);
66-
if(consRef!=null)
67-
cons = consRef.get();
68-
if(cons==null) {
69-
if (System.getSecurityManager() == null) {
70-
cons = tryGetDeclaredConstructor(clazz);
71-
} else {
72-
cons = AccessController.doPrivileged(new PrivilegedAction<>() {
73-
@Override
74-
public Constructor<T> run() {
75-
return tryGetDeclaredConstructor(clazz);
76-
}
77-
});
78-
}
79-
80-
int classMod = clazz.getModifiers();
81-
82-
if(!Modifier.isPublic(classMod) || !Modifier.isPublic(cons.getModifiers())) {
83-
// attempt to make it work even if the constructor is not accessible
84-
try {
85-
cons.setAccessible(true);
86-
} catch(SecurityException e) {
87-
// but if we don't have a permission to do so, work gracefully.
88-
logger.log(Level.FINE,"Unable to make the constructor of "+clazz+" accessible",e);
89-
throw e;
90-
}
91-
}
92-
93-
constructorClassCache.put(clazz, new WeakReference<>(cons));
94-
}
95-
87+
Constructor<T> cons = DECLARED_CTORS.get(clazz);
9688
return cons.newInstance(emptyObject);
9789
}
9890

9991
private static <T> Constructor<T> tryGetDeclaredConstructor(Class<T> clazz) {
10092
try {
10193
return clazz.getDeclaredConstructor(emptyClass);
10294
} catch (NoSuchMethodException e) {
103-
logger.log(Level.INFO,"No default constructor found on "+clazz,e);
95+
logger.log(Level.INFO, e, () -> "No default constructor found on "+clazz);
10496
NoSuchMethodError exp;
10597
if(clazz.getDeclaringClass()!=null && !Modifier.isStatic(clazz.getModifiers())) {
10698
exp = new NoSuchMethodError(Messages.NO_DEFAULT_CONSTRUCTOR_IN_INNER_CLASS
@@ -121,10 +113,10 @@ public static <T> T create( Class<T> clazz ) {
121113
try {
122114
return create0(clazz);
123115
} catch (InstantiationException e) {
124-
logger.log(Level.INFO,"failed to create a new instance of "+clazz,e);
116+
logger.log(Level.INFO, e, () -> "failed to create a new instance of "+clazz);
125117
throw new InstantiationError(e.toString());
126118
} catch (IllegalAccessException e) {
127-
logger.log(Level.INFO,"failed to create a new instance of "+clazz,e);
119+
logger.log(Level.INFO, e, () -> "failed to create a new instance of "+clazz);
128120
throw new IllegalAccessError(e.toString());
129121
} catch (InvocationTargetException e) {
130122
Throwable target = e.getTargetException();
@@ -163,10 +155,10 @@ public static Object create(Method method) {
163155

164156
throw new IllegalStateException(target);
165157
} catch (IllegalAccessException e) {
166-
logger.log(Level.INFO,"failed to create a new instance of "+method.getReturnType().getName(),e);
158+
logger.log(Level.INFO, e, () -> "failed to create a new instance of "+method.getReturnType().getName());
167159
throw new IllegalAccessError(e.toString());
168160
} catch (IllegalArgumentException | NullPointerException | ExceptionInInitializerError iae){
169-
logger.log(Level.INFO,"failed to create a new instance of "+method.getReturnType().getName(),iae);
161+
logger.log(Level.INFO, iae, () -> "failed to create a new instance of "+method.getReturnType().getName());
170162
errorMsg = iae;
171163
}
172164

@@ -175,9 +167,9 @@ public static Object create(Method method) {
175167
exp.initCause(errorMsg);
176168
throw exp;
177169
}
178-
170+
179171
/**
180-
* Infers the instanciable implementation class that can be assigned to the given field type.
172+
* Infers the instantiable implementation class that can be assigned to the given field type.
181173
*
182174
* @return null
183175
* if inference fails.

0 commit comments

Comments
 (0)