Skip to content

Commit a23083c

Browse files
committed
Fix classloading in ConstructorCache
Use the SDK standard ClassLoaderHelper#getClass to try and load the given class from a ClassLoader. Previously, we were only looking at the ClassLoader returned by ClassLoaderHelper#contextClassLoader which either returns the Thread's context ClassLoader, or the system ClassLoader if that isn't present. This is an issue because it's possible that neither of these ClassLoaders are correct; we should also be looking at the ClassLoader that loaded the calling class. As part of this change, the internal Map used in ConstructorCache has been changed. Rather than doing a two-level mapping from String -> (Map of ClassLoader to Class), we have a single level mapping from String -> Class. Class is still referenced via a WeakReference so it is still free to be GC'd at any point.
1 parent ee827e1 commit a23083c

File tree

1 file changed

+17
-15
lines changed

1 file changed

+17
-15
lines changed

core/checksums/src/main/java/software/amazon/awssdk/checksums/internal/ConstructorCache.java

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,25 @@
1717

1818
import java.lang.ref.WeakReference;
1919
import java.lang.reflect.Constructor;
20-
import java.util.Collections;
2120
import java.util.Map;
2221
import java.util.Optional;
23-
import java.util.WeakHashMap;
2422
import java.util.concurrent.ConcurrentHashMap;
2523
import software.amazon.awssdk.annotations.SdkInternalApi;
2624
import software.amazon.awssdk.utils.ClassLoaderHelper;
2725
import software.amazon.awssdk.utils.Logger;
2826

2927
/**
30-
* A cache that stores classes and their constructors by class name and class loader.
28+
* A cache that stores classes and exposes a method to retrieve its zero-argument constructor.
3129
* <p>
32-
* This cache uses weak references to both class loaders and classes, allowing them to be garbage collected
33-
* when no longer needed. It provides methods to retrieve the zero-argument constructor for a class,
34-
* based on the current thread's context class loader or the system class loader.
30+
* This cache stores weak references to loaded classes, allowing them to be garbage collected at any point.
31+
* <p>
32+
* Classes are loaded by first attempting to load it via the thread context {@code ClassLoader} (or system {@code ClassLoader} if
33+
* the calling thread does not have one). If that fails, will attempt using the {@code ClassLoader} that loaded this class, and
34+
* finally, attempt using the class that loaded {@link ClassLoaderHelper}.
3535
* <p>
3636
* If a class or its zero-argument constructor cannot be found, an empty result is returned.
37+
*
38+
* @see ClassLoaderHelper#loadClass(String, Class[])
3739
*/
3840
@SdkInternalApi
3941
public final class ConstructorCache {
@@ -43,36 +45,36 @@ public final class ConstructorCache {
4345
* Cache storing classes by class name and class loader.
4446
* Uses weak references to allow garbage collection when not needed.
4547
*/
46-
private final Map<String, Map<ClassLoader, Optional<WeakReference<Class<?>>>>> classesByClassName =
48+
private final Map<String, Optional<WeakReference<Class<?>>>> classesByClassName =
4749
new ConcurrentHashMap<>();
4850

4951
/**
5052
* Retrieve the class for the given class name from the context or system class loader.
5153
* Returns an empty result if the class is not found.
5254
*/
5355
private Optional<Class<?>> getClass(String className) {
54-
Map<ClassLoader, Optional<WeakReference<Class<?>>>> classesByClassLoader =
55-
classesByClassName.computeIfAbsent(className, k -> Collections.synchronizedMap(new WeakHashMap<>()));
56-
57-
ClassLoader classLoader = ClassLoaderHelper.contextClassLoader();
58-
Optional<WeakReference<Class<?>>> classRef = classesByClassLoader.computeIfAbsent(classLoader, k -> {
56+
Optional<WeakReference<Class<?>>> classRef = classesByClassName.computeIfAbsent(className, k -> {
5957
try {
60-
Class<?> clazz = classLoader.loadClass(className);
58+
Class<?> clazz = ClassLoaderHelper.loadClass(k, false);
6159
return Optional.of(new WeakReference<>(clazz));
6260
} catch (ClassNotFoundException e) {
6361
return Optional.empty();
6462
}
6563
});
6664

67-
// if the WeakReference to the class has been garbage collected, remove it from the cache and try again
65+
// Were we able to find this class?
6866
if (classRef.isPresent()) {
6967
Class<?> clazz = classRef.get().get();
68+
// Class hasn't been GC'd
7069
if (clazz != null) {
7170
return Optional.of(clazz);
7271
}
73-
classesByClassLoader.remove(classLoader);
72+
// if the WeakReference to the class has been garbage collected, it has been unloaded.
73+
// Remove it from the cache and try a fresh load
74+
classesByClassName.remove(className);
7475
return getClass(className);
7576
}
77+
7678
return Optional.empty();
7779
}
7880

0 commit comments

Comments
 (0)