-
Notifications
You must be signed in to change notification settings - Fork 198
[JENKINS-70869] More efficient JarURLConnection implementation
#673
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
Conversation
dwnusbaum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth adding a regression test for whatever behavior you are trying to prevent?
|
jenkinsci/pipeline-groovy-lib-plugin#57 effectively does that—without jglick/pipeline-groovy-lib-plugin@b7bd774 you get Windows failures in one test: https://github.com/jenkinsci/pipeline-groovy-lib-plugin/runs/11808228071 which I could reliably reproduce locally on Windows 10, and also on Linux if I used openjdk/jdk#12871 also fixes the leak, though I think this override is still potentially helpful in that it bypasses the need to ever open the JAR file to begin with in two cases used by Groovy (checking for a content encoding and checking for a last modification time). |
JarURLConnection implementationJarURLConnection implementation
|
@jocwurs moving thread from f8b1e63#commitcomment-109435305: not sure how you got into this situation (are you |
|
@jglick yes we are grapbing a java library which contains log4j2. Still do not fully understand what is going wrong here. |
|
@jocwurs looks like this is a known bug LOG4J2-3047, which has already been fixed as apache/logging-log4j2#1164. (BTW hi @jvz!) It may suffice to |
|
@jglick thanks for the hint to this log4j bug. The library was using another log4j version than expected. It took the log4j lib coming from the analysis-model-api-plugin. And this version contains an older log4j (2.19.0). |
jenkinsci/pipeline-groovy-lib-plugin#57 (comment)