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

[JENKINS-68417] Revert LogRecord tricks #376

Merged
merged 3 commits into from
Jul 14, 2022

Conversation

jglick
Copy link
Member

@jglick jglick commented Jun 9, 2022

Essentially reverting #324 (slight simpification to timing of format in the process). Essentially the same fix as jenkinsci/jenkins#6643. Here the calculus is simpler since any log records appearing in the handler will need to be formatted (we write them to disk files) so there is no justification for deferring the message format expansion.

Testing in pipeline-groovy-lib-plugin:

diff --git pom.xml pom.xml
index 9ee4e40..98f9e92 100644
--- pom.xml
+++ pom.xml
@@ -63,15 +63,15 @@
     </pluginRepositories>
     <properties>
         <changelist>999999-SNAPSHOT</changelist>
-        <jenkins.version>2.289.1</jenkins.version>
+        <jenkins.version>2.355-SNAPSHOT</jenkins.version> <!-- TODO https://github.com/jenkinsci/jenkins/pull/6643 -->
         <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
     </properties>
     <dependencyManagement>
         <dependencies>
             <dependency>
                 <groupId>io.jenkins.tools.bom</groupId>
-                <artifactId>bom-2.289.x</artifactId>
-                <version>1008.vb9e22885c9cf</version>
+                <artifactId>bom-2.332.x</artifactId>
+                <version>1342.v729ca_3818e88</version>
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
@@ -203,5 +203,11 @@
             <version>1.10.1</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>support-core</artifactId>
+            <version>999999-SNAPSHOT</version> <!-- TODO https://github.com/jenkinsci/support-core-plugin/pull/376 -->
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>
diff --git src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryMemoryTest.java src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryMemoryTest.java
index 59c9d05..dad9e09 100644
--- src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryMemoryTest.java
+++ src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryMemoryTest.java
@@ -26,6 +26,7 @@ package org.jenkinsci.plugins.workflow.libs;
 
 import groovy.lang.MetaClass;
 import hudson.PluginManager;
+import hudson.WebAppMain;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -65,6 +66,9 @@ public class LibraryMemoryTest {
     }
     @Issue("JENKINS-50223")
     @Test public void loaderReleased() throws Exception {
+        Method installLogger = WebAppMain.class.getDeclaredMethod("installLogger");
+        installLogger.setAccessible(true);
+        installLogger.invoke(new WebAppMain());
         sampleRepo.init();
         sampleRepo.write("src/p/C.groovy", "package p; class C {}");
         sampleRepo.write("vars/leak.groovy", "def call() {def c = node {new p.C()}; [this, c].each {" + LibraryMemoryTest.class.getName() + ".register(it)}}");

@jglick jglick requested a review from a team as a code owner June 9, 2022 21:21
@jglick jglick marked this pull request as draft June 12, 2022 12:32
@jglick
Copy link
Member Author

jglick commented Jun 16, 2022

(In draft as per jenkinsci/jenkins#6643 (comment).)

@jglick jglick changed the title [JENKINS-68417] Eagerly format LogRecord rather than holding SoftReferences [JENKINS-68417] Revert LogRecord tricks Jun 28, 2022
@jglick jglick marked this pull request as ready for review June 28, 2022 21:12
@jglick jglick requested a review from Dohbedoh July 6, 2022 12:53
@jglick jglick added the bug label Jul 6, 2022
@Dohbedoh Dohbedoh merged commit 46f7cc6 into jenkinsci:master Jul 14, 2022
@jglick jglick deleted the LogRecord-JENKINS-68417 branch July 14, 2022 14:29
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.

2 participants