You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/02/01 12:32:13 UTC
[GitHub] [skywalking] gaoweijie opened a new issue #6301: custom-enhance-plugin bug
gaoweijie opened a new issue #6301:
URL: https://github.com/apache/skywalking/issues/6301
Please answer these questions before submitting your issue.
- Why do you submit this issue?
- [ ] Question or discussion
- [ ] Bug
___
### Question
- Q1: There is a bug in version 8.3 of SW. If contextmanager creates span but does not execute "ContextManager.stopSpan ()", then threadLocal object's remove method will be skiped, there maybe existed a risk of ThreadLocal memory leak.
If so, there is a great risk:
In the customize-enhance-plugin, there is a set of configuration: close_ before_ method & close_ after_ method.
If user set the field close_before_method=true，close_ after_ Method = false, this maybe cause "ContextManager.stopSpan ()" don't execute, and then ThreadLocal doesn't execute the remove method. After a long time, there will be a memory leak, right?
- Q2: What is the meaning of these two parameters(close_before_method & close_ after_ Method)? Is it necessary to exist?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] gaoweijie commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
gaoweijie commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770831045
> Could you share which class(es) are you referring?
>
> About the leak, it may not leak, but the agent wouldn't work, and cost some but limited extra memory.
referring : BaseInterceptorMethods class -->
void afterMethod(Method method) {
if (!MethodConfiguration.isCloseAfterMethod(CustomizeConfiguration.INSTANCE.getConfiguration(method))) {
ContextManager.stopSpan();
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] gaoweijie commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
gaoweijie commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770851858
> I just did a direct call with @zhaoyuguang
>
> @gaoweijie I think these 2 parameters are the case of over-design, which makes things too complex. We agree, we should remove these 2 parameters, and also create span before method, and stop after that.
> Do you have interest to do this and send a pull request to upstream?
Yes, I am glad to fix this risk.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770851144
I just did a direct call with @zhaoyuguang
@gaoweijie I think these 2 parameters are the case of over-design, which makes things too complex. We agree, we should remove these 2 parameters, and also create span before method, and stop after that.
Do you have interest to do this and send a pull request to upstream?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] gaoweijie commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
gaoweijie commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770830065
Yes, generally speaking, it only takes up more memory. Moreover, if the configuration of the custom enhance plugin is not at the end of the call chain, it will have the opportunity to be removed by the after method of other plugins.
But it would be better if the risk was fixed.
In addition, I think this configuration of close_ after_ method can be removed, after all, close_ after_ Method does nothing special except to release the object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770823915
Could you share which class(es) are you referring?
About the leak, it may not leak, but the agent wouldn't work, and cost some but limited extra memory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] zhaoyuguang closed issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
zhaoyuguang closed issue #6301:
URL: https://github.com/apache/skywalking/issues/6301
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-792552068
No update of #6329, I change the status to `good first issue`, to see who wants to fix this
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770832019
@zhaoyuguang Could you take a look at this config? I am not sure about what are these configs for.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] zhaoyuguang commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
zhaoyuguang commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770853298
Now `Custom Plugin` is overdesign, I recommend that Custom Plugin only focus on synchronous threading model and Local Span. if that, createSpan and stopSpan are performed at the beginning of a method closure, the ML risk is lower and the configuration is easier
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] gaoweijie commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
gaoweijie commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770850653
OK, Thanks!
@wu-sheng @zhaoyuguang
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] zhaoyuguang commented on issue #6301: custom-enhance-plugin bug
Posted by GitBox <gi...@apache.org>.
zhaoyuguang commented on issue #6301:
URL: https://github.com/apache/skywalking/issues/6301#issuecomment-770847599
Q1:Yes, On a synchronous thread，createSpan count > stopSpan count will be ML.
Q2: `close_before_method` mean : the timing of method execution, before enhance method
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org