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.

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

add line before binding and it's related #27

Closed
wants to merge 7 commits into from

Conversation

isadliliying
Copy link
Contributor

@isadliliying isadliliying commented Jan 14, 2023

第一次提交PR,有描述不清楚的地方还请见谅并沟通,thanks

两个主要改动:

  1. Location基类中增加了filtered的标记,代表该LocationMatcher处理后,是否被LocationFilter过滤掉

  2. 添加BeforeLine类型的Binding及相关的Matcher

为什么需要暴露是否被过滤掉的状态呢?

  • 因为在使用行来进行增强(InterceptorProcessor#process)时发现如果没有明确出 匹配失败 和 被过滤 两种状态的话,在外层逻辑上没法做对应的处理(Arthas中无法判定是否需要注册listener)

为什么需要定义一个BeforeLine呢?用Line不行嘛?

  • 场景: 期待在Arthas获取到本地变量,选择的实现方式是在某行之前进行插桩
  • Binding时行号传递: Line使用了LineNumberNode中的行号,当我需要在 方法退出前 进行插入时,这边使用了-1来代表这个位置,所以Line无法给到-1这个值,在注册监听器时,这是个问题
  • LocationFilter过滤问题: LineLocationMatcher目前是没有应用过滤的,也就是会导致重复增强
  • 行号重复: 目前在增强kotlin类的时候发现,kotlin编译出来的class文件是存在重复行号问题的,使用LineLocationMatcher时候会尝试增强多个相同行号,LineBeforeLocationMatcher通过只取最前的一个来规避这个问题

thanks again

@CLAassistant
Copy link

CLAassistant commented Jan 14, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ isadliliying
❌ liliying


liliying seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@isadliliying isadliliying changed the title 1.添加BeforeLine类型的Location处理 add line before binding and related Jan 14, 2023
@isadliliying isadliliying changed the title add line before binding and related add line before binding and it's related Jan 14, 2023
@isadliliying
Copy link
Contributor Author

change username and email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants