This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-java.git
The following commit(s) were added to refs/heads/main by this push:
new 762d31fa0f Fix CLASS_LOADER_TYPE_CACHE OOM issue with WeakHashMap
(#772)
762d31fa0f is described below
commit 762d31fa0f462c3025adf6af2d862e52c37f6815
Author: Gong Dewei <kyl...@qq.com>
AuthorDate: Mon Sep 15 17:19:57 2025 +0800
Fix CLASS_LOADER_TYPE_CACHE OOM issue with WeakHashMap (#772)
---
CHANGES.md | 1 +
.../agent/builder/SWDescriptionStrategy.java | 26 ++-
.../builder/SWDescriptionStrategyCacheTest.java | 239 +++++++++++++++++++++
3 files changed, 260 insertions(+), 6 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index f28bb6123e..731f74226a 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -15,6 +15,7 @@ Release Notes.
* Bump up apache parent pom to v35.
* Update Maven to 3.6.3 in mvnw.
* Fix OOM due to too many span logs.
+* Fix ClassLoader cache OOM issue with WeakHashMap.
All issues and pull requests are
[here](https://github.com/apache/skywalking/milestone/242?closed=1)
diff --git
a/apm-sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java
b/apm-sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java
index 62e8a0606c..0d10a039bb 100644
---
a/apm-sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java
+++
b/apm-sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java
@@ -34,10 +34,12 @@ import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedI
import java.io.Serializable;
import java.lang.annotation.Annotation;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
@@ -125,9 +127,16 @@ public class SWDescriptionStrategy implements
AgentBuilder.DescriptionStrategy {
/**
* Original type cache.
- * classloader hashcode -> ( typeName -> type cache )
+ * ClassLoader -> (typeName -> TypeCache)
+ * Using WeakHashMap to automatically clean up cache when ClassLoader
is garbage collected.
*/
- private static final Map<Integer, Map<String, TypeCache>>
CLASS_LOADER_TYPE_CACHE = new ConcurrentHashMap<>();
+ private static final Map<ClassLoader, Map<String, TypeCache>>
CLASS_LOADER_TYPE_CACHE =
+ Collections.synchronizedMap(new WeakHashMap<>());
+
+ /**
+ * Bootstrap ClassLoader cache (null key cannot be stored in
WeakHashMap)
+ */
+ private static final Map<String, TypeCache> BOOTSTRAP_TYPE_CACHE = new
ConcurrentHashMap<>();
private static final List<String> IGNORED_INTERFACES =
Arrays.asList(EnhancedInstance.class.getName());
@@ -153,10 +162,15 @@ public class SWDescriptionStrategy implements
AgentBuilder.DescriptionStrategy {
}
private TypeCache getTypeCache() {
- int classLoaderHashCode = classLoader != null ?
classLoader.hashCode() : 0;
- Map<String, TypeCache> typeCacheMap =
CLASS_LOADER_TYPE_CACHE.computeIfAbsent(classLoaderHashCode, k -> new
ConcurrentHashMap<>());
- TypeCache typeCache = typeCacheMap.computeIfAbsent(typeName, k ->
new TypeCache(typeName));
- return typeCache;
+ if (classLoader == null) {
+ // Bootstrap ClassLoader - use separate cache since null key
cannot be stored in WeakHashMap
+ return BOOTSTRAP_TYPE_CACHE.computeIfAbsent(typeName, k -> new
TypeCache(typeName));
+ } else {
+ // Regular ClassLoader - use WeakHashMap for automatic cleanup
+ Map<String, TypeCache> typeCacheMap =
CLASS_LOADER_TYPE_CACHE.computeIfAbsent(
+ classLoader, k -> new ConcurrentHashMap<>());
+ return typeCacheMap.computeIfAbsent(typeName, k -> new
TypeCache(typeName));
+ }
}
@Override
diff --git
a/apm-sniffer/bytebuddy-patch/src/test/java/net/bytebuddy/agent/builder/SWDescriptionStrategyCacheTest.java
b/apm-sniffer/bytebuddy-patch/src/test/java/net/bytebuddy/agent/builder/SWDescriptionStrategyCacheTest.java
new file mode 100644
index 0000000000..90f9e4a437
--- /dev/null
+++
b/apm-sniffer/bytebuddy-patch/src/test/java/net/bytebuddy/agent/builder/SWDescriptionStrategyCacheTest.java
@@ -0,0 +1,239 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package net.bytebuddy.agent.builder;
+
+import org.junit.Test;
+import org.junit.Assert;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.net.URLClassLoader;
+import java.net.URL;
+import java.util.Map;
+
+/**
+ * Tests the behavior of the WeakHashMap caching mechanism in
SWDescriptionStrategy.
+ */
+public class SWDescriptionStrategyCacheTest {
+
+ @Test
+ public void testWeakHashMapCacheCleanup() throws Exception {
+ // Get static cache field
+ Field cacheField = SWDescriptionStrategy.SWTypeDescriptionWrapper.class
+ .getDeclaredField("CLASS_LOADER_TYPE_CACHE");
+ cacheField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>> cache =
+ (Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>>)
cacheField.get(null);
+
+ // Record initial cache size
+ int initialCacheSize = cache.size();
+
+ // Create test ClassLoader
+ URLClassLoader testClassLoader = new URLClassLoader(new URL[0], null);
+ String testTypeName = "com.test.DynamicClass";
+
+ // Create SWTypeDescriptionWrapper instance
+ SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
+ new SWDescriptionStrategy.SWTypeDescriptionWrapper(
+ null, "test", testClassLoader, testTypeName);
+
+ // Call getTypeCache method via reflection to trigger caching
+ Method getTypeCacheMethod = wrapper.getClass()
+ .getDeclaredMethod("getTypeCache");
+ getTypeCacheMethod.setAccessible(true);
+ SWDescriptionStrategy.TypeCache typeCache =
+ (SWDescriptionStrategy.TypeCache)
getTypeCacheMethod.invoke(wrapper);
+
+ // Verify that the ClassLoader exists in cache
+ Assert.assertTrue("Cache should contain the test ClassLoader",
+ cache.containsKey(testClassLoader));
+ Assert.assertNotNull("TypeCache should be created", typeCache);
+ Assert.assertEquals("Cache size should increase by 1",
+ initialCacheSize + 1, cache.size());
+
+ // Clear ClassLoader references, prepare for GC test
+ testClassLoader = null;
+ wrapper = null;
+ typeCache = null;
+
+ // Force garbage collection
+ System.gc();
+ Thread.sleep(100);
+ System.gc();
+ Thread.sleep(100);
+
+ // WeakHashMap should automatically clean up garbage collected
ClassLoader entries
+ int attempts = 0;
+ int currentCacheSize = cache.size();
+ while (currentCacheSize > initialCacheSize && attempts < 20) {
+ System.gc();
+ Thread.sleep(50);
+ currentCacheSize = cache.size();
+ attempts++;
+ }
+
+ System.out.println("Cache size after GC: " + currentCacheSize +
+ " (initial: " + initialCacheSize + ", attempts: " + attempts +
")");
+
+ // Verify that WeakHashMap cleanup mechanism works properly
+ Assert.assertTrue("WeakHashMap should clean up entries or attempts
should be reasonable",
+ currentCacheSize <= initialCacheSize + 1 && attempts < 20);
+ }
+
+ @Test
+ public void testBootstrapClassLoaderHandling() throws Exception {
+ // Get Bootstrap ClassLoader cache field
+ Field bootstrapCacheField =
SWDescriptionStrategy.SWTypeDescriptionWrapper.class
+ .getDeclaredField("BOOTSTRAP_TYPE_CACHE");
+ bootstrapCacheField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<String, SWDescriptionStrategy.TypeCache> bootstrapCache =
+ (Map<String, SWDescriptionStrategy.TypeCache>)
bootstrapCacheField.get(null);
+
+ int initialBootstrapCacheSize = bootstrapCache.size();
+
+ // Test Bootstrap ClassLoader (null) handling
+ String testTypeName = "test.BootstrapClass";
+ SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
+ new SWDescriptionStrategy.SWTypeDescriptionWrapper(
+ null, "test", null, testTypeName);
+
+ // Call getTypeCache method via reflection
+ Method getTypeCacheMethod = wrapper.getClass()
+ .getDeclaredMethod("getTypeCache");
+ getTypeCacheMethod.setAccessible(true);
+ SWDescriptionStrategy.TypeCache typeCache =
+ (SWDescriptionStrategy.TypeCache)
getTypeCacheMethod.invoke(wrapper);
+
+ // Verify Bootstrap ClassLoader cache handling
+ Assert.assertNotNull("TypeCache should be created for bootstrap
classloader", typeCache);
+ Assert.assertTrue("Bootstrap cache should contain the type",
+ bootstrapCache.containsKey(testTypeName));
+ Assert.assertEquals("Bootstrap cache size should increase by 1",
+ initialBootstrapCacheSize + 1, bootstrapCache.size());
+ }
+
+ @Test
+ public void testMultipleClassLoadersIndependentCache() throws Exception {
+ Field cacheField = SWDescriptionStrategy.SWTypeDescriptionWrapper.class
+ .getDeclaredField("CLASS_LOADER_TYPE_CACHE");
+ cacheField.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>> cache =
+ (Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>>)
cacheField.get(null);
+
+ int initialCacheSize = cache.size();
+
+ // Create two different ClassLoaders
+ URLClassLoader classLoader1 = new URLClassLoader(new URL[0], null);
+ URLClassLoader classLoader2 = new URLClassLoader(new URL[0], null);
+ String testTypeName = "com.test.SameClassName";
+
+ // Create TypeCache with same class name for both ClassLoaders
+ SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper1 =
+ new SWDescriptionStrategy.SWTypeDescriptionWrapper(
+ null, "test", classLoader1, testTypeName);
+ SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper2 =
+ new SWDescriptionStrategy.SWTypeDescriptionWrapper(
+ null, "test", classLoader2, testTypeName);
+
+ // Call getTypeCache method via reflection
+ Method getTypeCacheMethod =
+
SWDescriptionStrategy.SWTypeDescriptionWrapper.class.getDeclaredMethod("getTypeCache");
+ getTypeCacheMethod.setAccessible(true);
+
+ SWDescriptionStrategy.TypeCache typeCache1 =
+ (SWDescriptionStrategy.TypeCache)
getTypeCacheMethod.invoke(wrapper1);
+ SWDescriptionStrategy.TypeCache typeCache2 =
+ (SWDescriptionStrategy.TypeCache)
getTypeCacheMethod.invoke(wrapper2);
+
+ // Verify that the two ClassLoaders have independent cache entries
+ Assert.assertNotNull("TypeCache1 should be created", typeCache1);
+ Assert.assertNotNull("TypeCache2 should be created", typeCache2);
+ Assert.assertNotSame("TypeCaches should be different instances",
typeCache1, typeCache2);
+
+ // Verify cache structure
+ Assert.assertEquals("Cache should contain both classloaders",
+ initialCacheSize + 2, cache.size());
+ Assert.assertTrue("Cache should contain classloader1",
cache.containsKey(classLoader1));
+ Assert.assertTrue("Cache should contain classloader2",
cache.containsKey(classLoader2));
+
+ // Verify each ClassLoader has independent type cache
+ Map<String, SWDescriptionStrategy.TypeCache> typeCacheMap1 =
cache.get(classLoader1);
+ Map<String, SWDescriptionStrategy.TypeCache> typeCacheMap2 =
cache.get(classLoader2);
+
+ Assert.assertNotNull("ClassLoader1 should have type cache map",
typeCacheMap1);
+ Assert.assertNotNull("ClassLoader2 should have type cache map",
typeCacheMap2);
+ Assert.assertNotSame("Type cache maps should be different",
typeCacheMap1, typeCacheMap2);
+
+ Assert.assertTrue("ClassLoader1 cache should contain the type",
+ typeCacheMap1.containsKey(testTypeName));
+ Assert.assertTrue("ClassLoader2 cache should contain the type",
+ typeCacheMap2.containsKey(testTypeName));
+ }
+
+ @Test
+ public void testConcurrentAccess() throws Exception {
+ // Test concurrent access scenario
+ final String testTypeName = "com.test.ConcurrentClass";
+ final int threadCount = 10;
+ final Thread[] threads = new Thread[threadCount];
+ final URLClassLoader[] classLoaders = new URLClassLoader[threadCount];
+ final SWDescriptionStrategy.TypeCache[] results = new
SWDescriptionStrategy.TypeCache[threadCount];
+
+ // Create multiple threads to access cache simultaneously
+ for (int i = 0; i < threadCount; i++) {
+ final int index = i;
+ classLoaders[i] = new URLClassLoader(new URL[0], null);
+ threads[i] = new Thread(() -> {
+ try {
+ SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
+ new SWDescriptionStrategy.SWTypeDescriptionWrapper(
+ null, "test", classLoaders[index], testTypeName);
+
+ Method getTypeCacheMethod = wrapper.getClass()
+ .getDeclaredMethod("getTypeCache");
+ getTypeCacheMethod.setAccessible(true);
+ results[index] = (SWDescriptionStrategy.TypeCache)
+ getTypeCacheMethod.invoke(wrapper);
+ } catch (Exception e) {
+ Assert.fail("Concurrent access should not throw exception:
" + e.getMessage());
+ }
+ });
+ }
+
+ // Start all threads
+ for (Thread thread : threads) {
+ thread.start();
+ }
+
+ // Wait for all threads to complete
+ for (Thread thread : threads) {
+ thread.join(1000); // Wait at most 1 second
+ }
+
+ // Verify all results
+ for (int i = 0; i < threadCount; i++) {
+ Assert.assertNotNull("Result " + i + " should not be null",
results[i]);
+ }
+
+ System.out.println("Concurrent access test completed successfully with
" + threadCount + " threads");
+ }
+}
\ No newline at end of file
