Skip to content

fix: avoid same-thread nested advice dispatch in SpyImpl #3154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wl2027 wants to merge 1 commit into
base: master
Choose a base branch
from
+260 −54
Open

fix: avoid same-thread nested advice dispatch in SpyImpl #3154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 95 additions & 54 deletions core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
*/
public class SpyImpl extends AbstractSpy {
private static final Logger logger = LoggerFactory.getLogger(SpyImpl.class);
/**
* 防止 advice 回调链路里再次命中增强点时发生同线程递归分发。
*/
private static final ThreadLocal<Integer> DISPATCH_DEPTH = new ThreadLocal<Integer>();

@Override
public void atEnter(Class<?> clazz, String methodInfo, Object target, Object[] args) {
Expand All @@ -34,16 +38,20 @@ public void atEnter(Class<?> clazz, String methodInfo, Object target, Object[] a
// TODO listener 只用查一次，放到 thread local里保存起来就可以了！
List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
methodName, methodDesc);
if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
adviceListener.before(clazz, methodName, methodDesc, target, args);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
adviceListener.before(clazz, methodName, methodDesc, target, args);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
} finally {
exitDispatch();
}
}

Expand All @@ -59,16 +67,20 @@ public void atExit(Class<?> clazz, String methodInfo, Object target, Object[] ar

List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
methodName, methodDesc);
if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
adviceListener.afterReturning(clazz, methodName, methodDesc, target, args, returnObject);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
adviceListener.afterReturning(clazz, methodName, methodDesc, target, args, returnObject);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
} finally {
exitDispatch();
}
}
}
Expand All @@ -83,16 +95,20 @@ public void atExceptionExit(Class<?> clazz, String methodInfo, Object target, Ob

List<AdviceListener> listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(),
methodName, methodDesc);
if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
adviceListener.afterThrowing(clazz, methodName, methodDesc, target, args, throwable);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
adviceListener.afterThrowing(clazz, methodName, methodDesc, target, args, throwable);
} catch (Throwable e) {
logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e);
}
} finally {
exitDispatch();
}
}
}
Expand All @@ -107,18 +123,22 @@ public void atBeforeInvoke(Class<?> clazz, String invokeInfo, Object target) {

List<AdviceListener> listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(),
owner, methodName, methodDesc);

if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
int tracingLineNumber = Integer.parseInt(info[3]);
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeBeforeTracing(classLoader, owner, methodName, methodDesc, tracingLineNumber);
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
final InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeBeforeTracing(classLoader, owner, methodName, methodDesc, Integer.parseInt(info[3]));
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
} finally {
exitDispatch();
}
}
}
Expand All @@ -132,18 +152,22 @@ public void atAfterInvoke(Class<?> clazz, String invokeInfo, Object target) {
String methodDesc = info[2];
List<AdviceListener> listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(),
owner, methodName, methodDesc);

if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
int tracingLineNumber = Integer.parseInt(info[3]);
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeAfterTracing(classLoader, owner, methodName, methodDesc, tracingLineNumber);
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
final InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeAfterTracing(classLoader, owner, methodName, methodDesc, Integer.parseInt(info[3]));
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
} finally {
exitDispatch();
}
}

Expand All @@ -159,22 +183,39 @@ public void atInvokeException(Class<?> clazz, String invokeInfo, Object target,

List<AdviceListener> listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(),
owner, methodName, methodDesc);

if (listeners != null) {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
int tracingLineNumber = Integer.parseInt(info[3]);
if (listeners != null && tryEnterDispatch()) {
try {
for (AdviceListener adviceListener : listeners) {
try {
if (skipAdviceListener(adviceListener)) {
continue;
}
InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeThrowTracing(classLoader, owner, methodName, methodDesc, tracingLineNumber);
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
final InvokeTraceable listener = (InvokeTraceable) adviceListener;
listener.invokeThrowTracing(classLoader, owner, methodName, methodDesc, Integer.parseInt(info[3]));
} catch (Throwable e) {
logger.error("class: {}, invokeInfo: {}", clazz.getName(), invokeInfo, e);
}
} finally {
exitDispatch();
}
}
}

private static boolean tryEnterDispatch() {
Integer depth = DISPATCH_DEPTH.get();
if (depth != null && depth.intValue() > 0) {
return false;
}
DISPATCH_DEPTH.set(Integer.valueOf(1));
return true;
}

private static void exitDispatch() {
DISPATCH_DEPTH.remove();
}

private static boolean skipAdviceListener(AdviceListener adviceListener) {
if (adviceListener instanceof ProcessAware) {
ProcessAware processAware = (ProcessAware) adviceListener;
Expand Down
Loading
Loading