Skip to content

Commit b552f81

Browse files
daguimudaguimu
committed
fix: add ThreadLocal reentrancy guard to SpyImpl to prevent recursive advice dispatch
When watching a toString() method, Arthas's own ObjectView.renderObject() calls toString() on the watched object to display results. This triggers the advice listener again on the same thread, causing infinite recursion. Fix by adding a ThreadLocal<Boolean> DISPATCHING guard to SpyImpl. When already dispatching advice on the current thread, re-entrant calls from the same thread are skipped, preventing recursion. Applies guard to atEnter(), atExit(), and atExceptionExit(). Zero overhead on non-recursive paths (single ThreadLocal read). Fixes #3083
1 parent 9e56f55 commit b552f81

File tree

1 file changed

+79
-49
lines changed
  • core/src/main/java/com/taobao/arthas/core/advisor

1 file changed

+79
-49
lines changed

‎core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java‎

Lines changed: 79 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,76 +24,106 @@
2424
public class SpyImpl extends AbstractSpy {
2525
private static final Logger logger = LoggerFactory.getLogger(SpyImpl.class);
2626

27+
/**
28+
* Thread-local reentrancy guard to prevent recursive advice dispatch.
29+
* When ObjectView renders an object and calls toString(), that toString()
30+
* call must not re-trigger advice listeners on the same thread.
31+
*/
32+
private static final ThreadLocal<Boolean> DISPATCHING = new ThreadLocal<Boolean>() {
33+
@Override
34+
protected Boolean initialValue() {
35+
return Boolean.FALSE;
36+
}
37+
};
38+
2739
@Override
2840
public void atEnter(Class<?> clazz, String methodInfo, Object target, Object[] args) {
29-
ClassLoader classLoader = clazz.getClassLoader();
30-
31-
String[] info = StringUtils.splitMethodInfo(methodInfo);
32-
String methodName = info[0];
33-
String methodDesc = info[1];
34-
// TODO listener 只用查一次，放到 thread local里保存起来就可以了！
35-
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
36-
methodName, methodDesc);
37-
if (listeners != null) {
38-
for (AdviceListener adviceListener : listeners) {
39-
try {
40-
if (skipAdviceListener(adviceListener)) {
41-
continue;
41+
if (DISPATCHING.get()) {
42+
return;
43+
}
44+
DISPATCHING.set(Boolean.TRUE);
45+
try {
46+
ClassLoader classLoader = clazz.getClassLoader();
47+
String[] info = StringUtils.splitMethodInfo(methodInfo);
48+
String methodName = info[0];
49+
String methodDesc = info[1];
50+
// TODO listener 只用查一次，放到 thread local里保存起来就可以了！
51+
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
52+
methodName, methodDesc);
53+
if (listeners != null) {
54+
for (AdviceListener adviceListener : listeners) {
55+
try {
56+
if (skipAdviceListener(adviceListener)) {
57+
continue;
58+
}
59+
adviceListener.before(clazz, methodName, methodDesc, target, args);
60+
} catch (Throwable e) {
61+
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
4262
}
43-
adviceListener.before(clazz, methodName, methodDesc, target, args);
44-
} catch (Throwable e) {
45-
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
4663
}
4764
}
65+
} finally {
66+
DISPATCHING.set(Boolean.FALSE);
4867
}
49-
5068
}
5169

5270
@Override
5371
public void atExit(Class<?> clazz, String methodInfo, Object target, Object[] args, Object returnObject) {
54-
ClassLoader classLoader = clazz.getClassLoader();
55-
56-
String[] info = StringUtils.splitMethodInfo(methodInfo);
57-
String methodName = info[0];
58-
String methodDesc = info[1];
59-
60-
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
61-
methodName, methodDesc);
62-
if (listeners != null) {
63-
for (AdviceListener adviceListener : listeners) {
64-
try {
65-
if (skipAdviceListener(adviceListener)) {
66-
continue;
72+
if (DISPATCHING.get()) {
73+
return;
74+
}
75+
DISPATCHING.set(Boolean.TRUE);
76+
try {
77+
ClassLoader classLoader = clazz.getClassLoader();
78+
String[] info = StringUtils.splitMethodInfo(methodInfo);
79+
String methodName = info[0];
80+
String methodDesc = info[1];
81+
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
82+
methodName, methodDesc);
83+
if (listeners != null) {
84+
for (AdviceListener adviceListener : listeners) {
85+
try {
86+
if (skipAdviceListener(adviceListener)) {
87+
continue;
88+
}
89+
adviceListener.afterReturning(clazz, methodName, methodDesc, target, args, returnObject);
90+
} catch (Throwable e) {
91+
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
6792
}
68-
adviceListener.afterReturning(clazz, methodName, methodDesc, target, args, returnObject);
69-
} catch (Throwable e) {
70-
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
7193
}
7294
}
95+
} finally {
96+
DISPATCHING.set(Boolean.FALSE);
7397
}
7498
}
7599

76100
@Override
77101
public void atExceptionExit(Class<?> clazz, String methodInfo, Object target, Object[] args, Throwable throwable) {
78-
ClassLoader classLoader = clazz.getClassLoader();
79-
80-
String[] info = StringUtils.splitMethodInfo(methodInfo);
81-
String methodName = info[0];
82-
String methodDesc = info[1];
83-
84-
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
85-
methodName, methodDesc);
86-
if (listeners != null) {
87-
for (AdviceListener adviceListener : listeners) {
88-
try {
89-
if (skipAdviceListener(adviceListener)) {
90-
continue;
102+
if (DISPATCHING.get()) {
103+
return;
104+
}
105+
DISPATCHING.set(Boolean.TRUE);
106+
try {
107+
ClassLoader classLoader = clazz.getClassLoader();
108+
String[] info = StringUtils.splitMethodInfo(methodInfo);
109+
String methodName = info[0];
110+
String methodDesc = info[1];
111+
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
112+
methodName, methodDesc);
113+
if (listeners != null) {
114+
for (AdviceListener adviceListener : listeners) {
115+
try {
116+
if (skipAdviceListener(adviceListener)) {
117+
continue;
118+
}
119+
adviceListener.afterThrowing(clazz, methodName, methodDesc, target, args, throwable);
120+
} catch (Throwable e) {
121+
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
91122
}
92-
adviceListener.afterThrowing(clazz, methodName, methodDesc, target, args, throwable);
93-
} catch (Throwable e) {
94-
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
95123
}
96124
}
125+
} finally {
126+
DISPATCHING.set(Boolean.FALSE);
97127
}
98128
}
99129

0 commit comments

Comments
 (0)