Skip to content
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

Jump to bottom

解决AdviceListener半路执行了after却没先执行before的问题，具体表现有：#cost计算错误 #2959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

解决AdviceListener半路执行了after却没先执行before的问题，具体表现有：#cost计算错误 #2959

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
97 changes: 46 additions & 51 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
@@ -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;
Expand All @@ -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<ArrayList<AdviceListener>> LISTENERS = ThreadLocal.withInitial(ArrayList::new);

@Override
public void atEnter(Class<?> clazz, String methodInfo, Object target, Object[] args) {
Expand All @@ -33,65 +37,60 @@ 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);

ArrayList<AdviceListener> 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<AdviceListener> 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<AdviceListener> 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<AdviceListener> 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<AdviceListener> 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);
}
}
}
Expand All @@ -107,12 +106,15 @@ public void atBeforeInvoke(Class<?> clazz, String invokeInfo, Object target) {
List<AdviceListener> listeners = AdviceListenerManager.queryTraceAdviceListeners(classLoader, clazz.getName(),
owner, methodName, methodDesc);

ArrayList<AdviceListener> 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) {
Expand All @@ -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<AdviceListener> 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<AdviceListener> 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);
}
}

Expand All @@ -156,20 +155,16 @@ public void atInvokeException(Class<?> clazz, String invokeInfo, Object target,
String methodName = info[1];
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;
}
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<AdviceListener> 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);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/com/taobao/arthas/core/command/model/TraceTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
8 changes: 8 additions & 0 deletions .../src/main/java/com/taobao/arthas/core/command/monitor200/AbstractTraceAdviceListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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();
Expand Down