Skip to content

Commit 762d31f

Browse files
kylixskylixs
authored
Fix CLASS_LOADER_TYPE_CACHE OOM issue with WeakHashMap (#772)
1 parent d1af3f6 commit 762d31f

File tree

3 files changed

+260
-6
lines changed

3 files changed

+260
-6
lines changed

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Release Notes.
1515
* Bump up apache parent pom to v35.
1616
* Update Maven to 3.6.3 in mvnw.
1717
* Fix OOM due to too many span logs.
18+
* Fix ClassLoader cache OOM issue with WeakHashMap.
1819

1920
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/242?closed=1)
2021

‎apm-sniffer/bytebuddy-patch/src/main/java/net/bytebuddy/agent/builder/SWDescriptionStrategy.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@
3434
import java.io.Serializable;
3535
import java.lang.annotation.Annotation;
3636
import java.util.Arrays;
37+
import java.util.Collections;
3738
import java.util.HashMap;
3839
import java.util.List;
3940
import java.util.Map;
4041
import java.util.Set;
42+
import java.util.WeakHashMap;
4143
import java.util.concurrent.ConcurrentHashMap;
4244
import java.util.stream.Collectors;
4345

@@ -125,9 +127,16 @@ static class SWTypeDescriptionWrapper extends TypeDescription.AbstractBase imple
125127

126128
/**
127129
* Original type cache.
128-
* classloader hashcode -> ( typeName -> type cache )
130+
* ClassLoader -> (typeName -> TypeCache)
131+
* Using WeakHashMap to automatically clean up cache when ClassLoader is garbage collected.
129132
*/
130-
private static final Map<Integer, Map<String, TypeCache>> CLASS_LOADER_TYPE_CACHE = new ConcurrentHashMap<>();
133+
private static final Map<ClassLoader, Map<String, TypeCache>> CLASS_LOADER_TYPE_CACHE =
134+
Collections.synchronizedMap(new WeakHashMap<>());
135+
136+
/**
137+
* Bootstrap ClassLoader cache (null key cannot be stored in WeakHashMap)
138+
*/
139+
private static final Map<String, TypeCache> BOOTSTRAP_TYPE_CACHE = new ConcurrentHashMap<>();
131140

132141
private static final List<String> IGNORED_INTERFACES = Arrays.asList(EnhancedInstance.class.getName());
133142

@@ -153,10 +162,15 @@ public SWTypeDescriptionWrapper(TypeDescription delegate, String nameTrait, Clas
153162
}
154163

155164
private TypeCache getTypeCache() {
156-
int classLoaderHashCode = classLoader != null ? classLoader.hashCode() : 0;
157-
Map<String, TypeCache> typeCacheMap = CLASS_LOADER_TYPE_CACHE.computeIfAbsent(classLoaderHashCode, k -> new ConcurrentHashMap<>());
158-
TypeCache typeCache = typeCacheMap.computeIfAbsent(typeName, k -> new TypeCache(typeName));
159-
return typeCache;
165+
if (classLoader == null) {
166+
// Bootstrap ClassLoader - use separate cache since null key cannot be stored in WeakHashMap
167+
return BOOTSTRAP_TYPE_CACHE.computeIfAbsent(typeName, k -> new TypeCache(typeName));
168+
} else {
169+
// Regular ClassLoader - use WeakHashMap for automatic cleanup
170+
Map<String, TypeCache> typeCacheMap = CLASS_LOADER_TYPE_CACHE.computeIfAbsent(
171+
classLoader, k -> new ConcurrentHashMap<>());
172+
return typeCacheMap.computeIfAbsent(typeName, k -> new TypeCache(typeName));
173+
}
160174
}
161175

162176
@Override

‎apm-sniffer/bytebuddy-patch/src/test/java/net/bytebuddy/agent/builder/SWDescriptionStrategyCacheTest.java‎

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package net.bytebuddy.agent.builder;
20+
21+
import org.junit.Test;
22+
import org.junit.Assert;
23+
24+
import java.lang.reflect.Field;
25+
import java.lang.reflect.Method;
26+
import java.net.URLClassLoader;
27+
import java.net.URL;
28+
import java.util.Map;
29+
30+
/**
31+
* Tests the behavior of the WeakHashMap caching mechanism in SWDescriptionStrategy.
32+
*/
33+
public class SWDescriptionStrategyCacheTest {
34+
35+
@Test
36+
public void testWeakHashMapCacheCleanup() throws Exception {
37+
// Get static cache field
38+
Field cacheField = SWDescriptionStrategy.SWTypeDescriptionWrapper.class
39+
.getDeclaredField("CLASS_LOADER_TYPE_CACHE");
40+
cacheField.setAccessible(true);
41+
@SuppressWarnings("unchecked")
42+
Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>> cache =
43+
(Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>>) cacheField.get(null);
44+
45+
// Record initial cache size
46+
int initialCacheSize = cache.size();
47+
48+
// Create test ClassLoader
49+
URLClassLoader testClassLoader = new URLClassLoader(new URL[0], null);
50+
String testTypeName = "com.test.DynamicClass";
51+
52+
// Create SWTypeDescriptionWrapper instance
53+
SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
54+
new SWDescriptionStrategy.SWTypeDescriptionWrapper(
55+
null, "test", testClassLoader, testTypeName);
56+
57+
// Call getTypeCache method via reflection to trigger caching
58+
Method getTypeCacheMethod = wrapper.getClass()
59+
.getDeclaredMethod("getTypeCache");
60+
getTypeCacheMethod.setAccessible(true);
61+
SWDescriptionStrategy.TypeCache typeCache =
62+
(SWDescriptionStrategy.TypeCache) getTypeCacheMethod.invoke(wrapper);
63+
64+
// Verify that the ClassLoader exists in cache
65+
Assert.assertTrue("Cache should contain the test ClassLoader",
66+
cache.containsKey(testClassLoader));
67+
Assert.assertNotNull("TypeCache should be created", typeCache);
68+
Assert.assertEquals("Cache size should increase by 1",
69+
initialCacheSize + 1, cache.size());
70+
71+
// Clear ClassLoader references, prepare for GC test
72+
testClassLoader = null;
73+
wrapper = null;
74+
typeCache = null;
75+
76+
// Force garbage collection
77+
System.gc();
78+
Thread.sleep(100);
79+
System.gc();
80+
Thread.sleep(100);
81+
82+
// WeakHashMap should automatically clean up garbage collected ClassLoader entries
83+
int attempts = 0;
84+
int currentCacheSize = cache.size();
85+
while (currentCacheSize > initialCacheSize && attempts < 20) {
86+
System.gc();
87+
Thread.sleep(50);
88+
currentCacheSize = cache.size();
89+
attempts++;
90+
}
91+
92+
System.out.println("Cache size after GC: " + currentCacheSize +
93+
" (initial: " + initialCacheSize + ", attempts: " + attempts + ")");
94+
95+
// Verify that WeakHashMap cleanup mechanism works properly
96+
Assert.assertTrue("WeakHashMap should clean up entries or attempts should be reasonable",
97+
currentCacheSize <= initialCacheSize + 1 && attempts < 20);
98+
}
99+
100+
@Test
101+
public void testBootstrapClassLoaderHandling() throws Exception {
102+
// Get Bootstrap ClassLoader cache field
103+
Field bootstrapCacheField = SWDescriptionStrategy.SWTypeDescriptionWrapper.class
104+
.getDeclaredField("BOOTSTRAP_TYPE_CACHE");
105+
bootstrapCacheField.setAccessible(true);
106+
@SuppressWarnings("unchecked")
107+
Map<String, SWDescriptionStrategy.TypeCache> bootstrapCache =
108+
(Map<String, SWDescriptionStrategy.TypeCache>) bootstrapCacheField.get(null);
109+
110+
int initialBootstrapCacheSize = bootstrapCache.size();
111+
112+
// Test Bootstrap ClassLoader (null) handling
113+
String testTypeName = "test.BootstrapClass";
114+
SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
115+
new SWDescriptionStrategy.SWTypeDescriptionWrapper(
116+
null, "test", null, testTypeName);
117+
118+
// Call getTypeCache method via reflection
119+
Method getTypeCacheMethod = wrapper.getClass()
120+
.getDeclaredMethod("getTypeCache");
121+
getTypeCacheMethod.setAccessible(true);
122+
SWDescriptionStrategy.TypeCache typeCache =
123+
(SWDescriptionStrategy.TypeCache) getTypeCacheMethod.invoke(wrapper);
124+
125+
// Verify Bootstrap ClassLoader cache handling
126+
Assert.assertNotNull("TypeCache should be created for bootstrap classloader", typeCache);
127+
Assert.assertTrue("Bootstrap cache should contain the type",
128+
bootstrapCache.containsKey(testTypeName));
129+
Assert.assertEquals("Bootstrap cache size should increase by 1",
130+
initialBootstrapCacheSize + 1, bootstrapCache.size());
131+
}
132+
133+
@Test
134+
public void testMultipleClassLoadersIndependentCache() throws Exception {
135+
Field cacheField = SWDescriptionStrategy.SWTypeDescriptionWrapper.class
136+
.getDeclaredField("CLASS_LOADER_TYPE_CACHE");
137+
cacheField.setAccessible(true);
138+
@SuppressWarnings("unchecked")
139+
Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>> cache =
140+
(Map<ClassLoader, Map<String, SWDescriptionStrategy.TypeCache>>) cacheField.get(null);
141+
142+
int initialCacheSize = cache.size();
143+
144+
// Create two different ClassLoaders
145+
URLClassLoader classLoader1 = new URLClassLoader(new URL[0], null);
146+
URLClassLoader classLoader2 = new URLClassLoader(new URL[0], null);
147+
String testTypeName = "com.test.SameClassName";
148+
149+
// Create TypeCache with same class name for both ClassLoaders
150+
SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper1 =
151+
new SWDescriptionStrategy.SWTypeDescriptionWrapper(
152+
null, "test", classLoader1, testTypeName);
153+
SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper2 =
154+
new SWDescriptionStrategy.SWTypeDescriptionWrapper(
155+
null, "test", classLoader2, testTypeName);
156+
157+
// Call getTypeCache method via reflection
158+
Method getTypeCacheMethod =
159+
SWDescriptionStrategy.SWTypeDescriptionWrapper.class.getDeclaredMethod("getTypeCache");
160+
getTypeCacheMethod.setAccessible(true);
161+
162+
SWDescriptionStrategy.TypeCache typeCache1 =
163+
(SWDescriptionStrategy.TypeCache) getTypeCacheMethod.invoke(wrapper1);
164+
SWDescriptionStrategy.TypeCache typeCache2 =
165+
(SWDescriptionStrategy.TypeCache) getTypeCacheMethod.invoke(wrapper2);
166+
167+
// Verify that the two ClassLoaders have independent cache entries
168+
Assert.assertNotNull("TypeCache1 should be created", typeCache1);
169+
Assert.assertNotNull("TypeCache2 should be created", typeCache2);
170+
Assert.assertNotSame("TypeCaches should be different instances", typeCache1, typeCache2);
171+
172+
// Verify cache structure
173+
Assert.assertEquals("Cache should contain both classloaders",
174+
initialCacheSize + 2, cache.size());
175+
Assert.assertTrue("Cache should contain classloader1", cache.containsKey(classLoader1));
176+
Assert.assertTrue("Cache should contain classloader2", cache.containsKey(classLoader2));
177+
178+
// Verify each ClassLoader has independent type cache
179+
Map<String, SWDescriptionStrategy.TypeCache> typeCacheMap1 = cache.get(classLoader1);
180+
Map<String, SWDescriptionStrategy.TypeCache> typeCacheMap2 = cache.get(classLoader2);
181+
182+
Assert.assertNotNull("ClassLoader1 should have type cache map", typeCacheMap1);
183+
Assert.assertNotNull("ClassLoader2 should have type cache map", typeCacheMap2);
184+
Assert.assertNotSame("Type cache maps should be different", typeCacheMap1, typeCacheMap2);
185+
186+
Assert.assertTrue("ClassLoader1 cache should contain the type",
187+
typeCacheMap1.containsKey(testTypeName));
188+
Assert.assertTrue("ClassLoader2 cache should contain the type",
189+
typeCacheMap2.containsKey(testTypeName));
190+
}
191+
192+
@Test
193+
public void testConcurrentAccess() throws Exception {
194+
// Test concurrent access scenario
195+
final String testTypeName = "com.test.ConcurrentClass";
196+
final int threadCount = 10;
197+
final Thread[] threads = new Thread[threadCount];
198+
final URLClassLoader[] classLoaders = new URLClassLoader[threadCount];
199+
final SWDescriptionStrategy.TypeCache[] results = new SWDescriptionStrategy.TypeCache[threadCount];
200+
201+
// Create multiple threads to access cache simultaneously
202+
for (int i = 0; i < threadCount; i++) {
203+
final int index = i;
204+
classLoaders[i] = new URLClassLoader(new URL[0], null);
205+
threads[i] = new Thread(() -> {
206+
try {
207+
SWDescriptionStrategy.SWTypeDescriptionWrapper wrapper =
208+
new SWDescriptionStrategy.SWTypeDescriptionWrapper(
209+
null, "test", classLoaders[index], testTypeName);
210+
211+
Method getTypeCacheMethod = wrapper.getClass()
212+
.getDeclaredMethod("getTypeCache");
213+
getTypeCacheMethod.setAccessible(true);
214+
results[index] = (SWDescriptionStrategy.TypeCache)
215+
getTypeCacheMethod.invoke(wrapper);
216+
} catch (Exception e) {
217+
Assert.fail("Concurrent access should not throw exception: " + e.getMessage());
218+
}
219+
});
220+
}
221+
222+
// Start all threads
223+
for (Thread thread : threads) {
224+
thread.start();
225+
}
226+
227+
// Wait for all threads to complete
228+
for (Thread thread : threads) {
229+
thread.join(1000); // Wait at most 1 second
230+
}
231+
232+
// Verify all results
233+
for (int i = 0; i < threadCount; i++) {
234+
Assert.assertNotNull("Result " + i + " should not be null", results[i]);
235+
}
236+
237+
System.out.println("Concurrent access test completed successfully with " + threadCount + " threads");
238+
}
239+
}

0 commit comments

Comments
 (0)