Skip to content

Conversation

@jglick
Copy link
Member

@jglick jglick commented Mar 6, 2023

@jglick jglick added the bug label Mar 6, 2023
jglick added a commit to jglick/pipeline-groovy-lib-plugin that referenced this pull request Mar 7, 2023
@jglick jglick marked this pull request as ready for review March 8, 2023 22:59
@jglick jglick requested a review from a team as a code owner March 8, 2023 22:59
@jglick jglick requested a review from dwnusbaum March 9, 2023 12:22
Copy link
Member

@dwnusbaum dwnusbaum left a 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?

@jglick
Copy link
Member Author

jglick commented Mar 9, 2023

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 file-leak-detector to assert that the lib JAR was not open at the end of the test. Theoretically it would be possible to write a JenkinsRule test here that fails (on Windows) without the hack but it would be pretty artificial since until jenkinsci/pipeline-groovy-lib-plugin#57 there would be no scenario (that I know of) in which Groovy sources are loaded from a JAR file.

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).

@jglick jglick merged commit 63bc17e into jenkinsci:master Mar 9, 2023
@jglick jglick deleted the JarURLConnection branch March 9, 2023 22:09
jglick added a commit to jglick/pipeline-groovy-lib-plugin that referenced this pull request Mar 9, 2023
@jglick jglick changed the title More efficient JarURLConnection implementation [JENKINS-70869] More efficient JarURLConnection implementation Mar 24, 2023
@jglick
Copy link
Member Author

jglick commented Apr 18, 2023

@jocwurs moving thread from f8b1e63#commitcomment-109435305: not sure how you got into this situation (are you @Grabbing Log4j or something?) but it sounds like org.apache.logging.log4j.core.config.ConfigurationSource is at fault.

@jocwurs
Copy link

jocwurs commented Apr 19, 2023

@jglick yes we are grapbing a java library which contains log4j2. Still do not fully understand what is going wrong here.

@jglick
Copy link
Member Author

jglick commented Apr 19, 2023

@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 @Grab the fixed release explicitly before the library which pulls in a broken version.

@jocwurs
Copy link

jocwurs commented Apr 24, 2023

@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).
BY shadowing log4j 2.20.0 I am now able to use the latest version of workflow-cps.
@uhafner maybe it is a good idea to update log4j used by analysis-model-api-plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants