From 7a46c1aff0c8acc62ae0c39049905d3954fe9af6 Mon Sep 17 00:00:00 2001 From: trytocatch Date: Fri, 22 Nov 2024 17:19:18 +0800 Subject: [PATCH] =?UTF-8?q?=E8=A7=A3=E5=86=B3AdviceListener=E5=8D=8A?= =?UTF-8?q?=E8=B7=AF=E6=89=A7=E8=A1=8C=E4=BA=86after=E5=8D=B4=E6=B2=A1?= =?UTF-8?q?=E5=85=88=E6=89=A7=E8=A1=8Cbefore=E7=9A=84=E9=97=AE=E9=A2=98?= =?UTF-8?q?=EF=BC=8C=E5=85=B7=E4=BD=93=E8=A1=A8=E7=8E=B0=E6=9C=89=EF=BC=9A?= =?UTF-8?q?#cost=E8=AE=A1=E7=AE=97=E9=94=99=E8=AF=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../taobao/arthas/core/advisor/SpyImpl.java | 97 +++++++++---------- .../arthas/core/command/model/TraceTree.java | 2 + .../AbstractTraceAdviceListener.java | 8 ++ 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java b/core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java index 8cf54962326..06b1a426216 100644 --- a/core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java +++ b/core/src/main/java/com/taobao/arthas/core/advisor/SpyImpl.java @@ -1,6 +1,7 @@ package com.taobao.arthas.core.advisor; import java.arthas.SpyAPI.AbstractSpy; +import java.util.ArrayList; import java.util.List; import com.alibaba.arthas.deps.org.slf4j.Logger; @@ -22,6 +23,9 @@ */ public class SpyImpl extends AbstractSpy { private static final Logger logger = LoggerFactory.getLogger(SpyImpl.class); + //将enter时执行过的AdviceListener暂存起来，exit时取出来，再执行过滤，避免对于同一次增强方法的调用，没有执行enter却执行了exit， + // 用null作为分隔符，尽量减少内存占用 + private static final ThreadLocal> LISTENERS = ThreadLocal.withInitial(ArrayList::new); @Override public void atEnter(Class clazz, String methodInfo, Object target, Object[] args) { @@ -33,65 +37,60 @@ public void atEnter(Class clazz, String methodInfo, Object target, Object[] a // TODO listener 只用查一次，放到 thread local里保存起来就可以了！ List listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(), methodName, methodDesc); + + ArrayList stack = LISTENERS.get(); + stack.add(null); if (listeners != null) { for (AdviceListener adviceListener : listeners) { try { if (skipAdviceListener(adviceListener)) { continue; } + stack.add(adviceListener); adviceListener.before(clazz, methodName, methodDesc, target, args); } catch (Throwable e) { logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e); } } } - } @Override public void atExit(Class clazz, String methodInfo, Object target, Object[] args, Object returnObject) { - ClassLoader classLoader = clazz.getClassLoader(); + //基于目前的增强实现，atExit方法中不能抛出异常，否则，atExit和atExceptionExit的代码可能被同时执行，造成逻辑错乱 String[] info = StringUtils.splitMethodInfo(methodInfo); String methodName = info[0]; String methodDesc = info[1]; - List listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(), - methodName, methodDesc); - if (listeners != null) { - 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); + ArrayList stack = LISTENERS.get(); + for (AdviceListener adviceListener; (adviceListener = stack.remove(stack.size() - 1)) != null; ) { + try { + if (skipAdviceListener(adviceListener)) { + continue; } + adviceListener.afterReturning(clazz, methodName, methodDesc, target, args, returnObject); + } catch (Throwable e) { + logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e); } } } @Override public void atExceptionExit(Class clazz, String methodInfo, Object target, Object[] args, Throwable throwable) { - ClassLoader classLoader = clazz.getClassLoader(); - String[] info = StringUtils.splitMethodInfo(methodInfo); String methodName = info[0]; String methodDesc = info[1]; - List listeners = AdviceListenerManager.queryAdviceListeners(classLoader, clazz.getName(), - methodName, methodDesc); - if (listeners != null) { - 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); + ArrayList stack = LISTENERS.get(); + for (AdviceListener adviceListener; (adviceListener = stack.remove(stack.size() - 1)) != null; ) { + try { + if (skipAdviceListener(adviceListener)) { + continue; } + adviceListener.afterThrowing(clazz, methodName, methodDesc, target, args, throwable); + } catch (Throwable e) { + logger.error("class: {}, methodInfo: {}", clazz.getName(), methodInfo, e); } } } @@ -107,12 +106,15 @@ public void atBeforeInvoke(Class clazz, String invokeInfo, Object target) { List listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(), owner, methodName, methodDesc); + ArrayList stack = LISTENERS.get(); + stack.add(null); if (listeners != null) { for (AdviceListener adviceListener : listeners) { try { if (skipAdviceListener(adviceListener)) { continue; } + stack.add(adviceListener); final InvokeTraceable listener = (InvokeTraceable) adviceListener; listener.invokeBeforeTracing(classLoader, owner, methodName, methodDesc, Integer.parseInt(info[3])); } catch (Throwable e) { @@ -129,20 +131,17 @@ public void atAfterInvoke(Class clazz, String invokeInfo, Object target) { String owner = info[0]; String methodName = info[1]; String methodDesc = info[2]; - List listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(), - owner, methodName, methodDesc); - if (listeners != null) { - for (AdviceListener adviceListener : listeners) { - try { - if (skipAdviceListener(adviceListener)) { - continue; - } - 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); + ArrayList stack = LISTENERS.get(); + for (AdviceListener adviceListener; (adviceListener = stack.remove(stack.size() - 1)) != null; ) { + try { + if (skipAdviceListener(adviceListener)) { + continue; } + 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); } } @@ -156,20 +155,16 @@ public void atInvokeException(Class clazz, String invokeInfo, Object target, String methodName = info[1]; String methodDesc = info[2]; - List listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(), - owner, methodName, methodDesc); - - if (listeners != null) { - for (AdviceListener adviceListener : listeners) { - try { - if (skipAdviceListener(adviceListener)) { - continue; - } - 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); + ArrayList stack = LISTENERS.get(); + for (AdviceListener adviceListener; (adviceListener = stack.remove(stack.size() - 1)) != null; ) { + try { + if (skipAdviceListener(adviceListener)) { + continue; } + 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); } } } diff --git a/core/src/main/java/com/taobao/arthas/core/command/model/TraceTree.java b/core/src/main/java/com/taobao/arthas/core/command/model/TraceTree.java index f4957346b76..d8c5ada9d7c 100644 --- a/core/src/main/java/com/taobao/arthas/core/command/model/TraceTree.java +++ b/core/src/main/java/com/taobao/arthas/core/command/model/TraceTree.java @@ -66,6 +66,8 @@ public void end() { current.end(); if (current.parent() != null) { //TODO 为什么会到达这里？ 调用end次数比begin多？ + // 因为AdviceListener中的before与after并不保证配对执行，那么此处便存在有end没有begin的情况， + // 此次修复后，不存在有after没有before的情况了，但仍存在不配对的情况，还有待进一步重构&完善 current = current.parent(); } } diff --git a/core/src/main/java/com/taobao/arthas/core/command/monitor200/AbstractTraceAdviceListener.java b/core/src/main/java/com/taobao/arthas/core/command/monitor200/AbstractTraceAdviceListener.java index e9eaee2f2bb..bb43fcc53b7 100644 --- a/core/src/main/java/com/taobao/arthas/core/command/monitor200/AbstractTraceAdviceListener.java +++ b/core/src/main/java/com/taobao/arthas/core/command/monitor200/AbstractTraceAdviceListener.java @@ -39,6 +39,8 @@ protected TraceEntity threadLocalTraceEntity(ClassLoader loader) { @Override public void destroy() { + //TODO 这里对ThreadLocal执行remove意义不大，不严谨，调用此方法的线程，跟执行before那些方法的线程可能不是一个线程， + // 比如执行q指令、ctrl+c指令的线程，既然线程都不是之前set值的那个线程了，那么对ThreadLocal执行remove便没意义了 threadBoundEntity.remove(); } @@ -83,6 +85,12 @@ private void finishing(ClassLoader loader, Advice advice) { TraceEntity traceEntity = threadLocalTraceEntity(loader); if (traceEntity.deep >= 1) { // #1817 防止deep为负数 traceEntity.deep--; + //此次修复了有after没有before的问题后，deep目前不会减为负数了， + //不过目前仍存在不配对的情况(即有before没有after，此情况目前是没问题的)， + //TODO 建议后续重构before与after的机制，将其与条件匹配&结果输出等逻辑剥离出来， + // 严格保证traceEntity.deep++与traceEntity.deep--等这些“进出栈”逻辑能配对执行，不管当前Process的状态如何， + // 进行条件判断&输出结果时再根据Process的状态来决定是否跳过。 + // 可能也是因为不配对的问题，导致很多ThreadLocal不能定义为static，需要在AdviceListener销毁的时候一同废弃 } if (traceEntity.deep == 0) { double cost = threadLocalWatch.costInMillis();