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-71970] Memory leak involving BufferedBuildListener #311

Merged
merged 3 commits into from Sep 13, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Sep 13, 2023

JENKINS-71970, amending #309.

Reproduced by running mvn hpi:run with

diff --git pom.xml pom.xml
index c4482a2..646fe12 100644
--- pom.xml
+++ pom.xml
@@ -63,7 +63,7 @@
     </pluginRepositories>
     <properties>
         <changelist>999999-SNAPSHOT</changelist>
-        <jenkins.version>2.361.4</jenkins.version>
+        <jenkins.version>2.414.1</jenkins.version>
         <no-test-jar>false</no-test-jar>
         <useBeta>true</useBeta>
         <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
@@ -72,8 +72,8 @@
         <dependencies>
             <dependency>
                 <groupId>io.jenkins.tools.bom</groupId>
-                <artifactId>bom-2.361.x</artifactId>
-                <version>2102.v854b_fec19c92</version>
+                <artifactId>bom-2.414.x</artifactId>
+                <version>2423.vce598171d115</version>
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>

plus updating workflow-support from UC and installing mock-slave, creating a mock slave, and running

node('static') {
    int i = 0
    while (true) {
        sh "echo ${i}"
        i++
    }
}

then

watch 'jmap -histo:live …

or

watch 'jmap -histo:live … | fgrep BufferedBuildListener'

There were multiple issues. First of all, removing the listener on Channel.onClosed is not enough (and may in fact be useless) when there are lots of different listeners being created while the channel is open. The listener needed to be associated with the stream (BufferedBuildListener), not its Replacement which is intended to be transient. Closing the stream (if that ever happens) should also remove the listener. And collecting the stream (which happens routinely) should remove the listener.

With the fix, I was able to run thousands of sh steps in a row on the same open agent channel, and memory did not increase significantly (in particular the byte[] usage would go up to ~10Mb but then drop again), nor did any classes from this package stay in the top screenful.

@jglick jglick requested a review from a team as a code owner September 13, 2023 18:09
@jglick jglick added the bug label Sep 13, 2023
Co-authored-by: Devin Nusbaum <dwnusbaum@users.noreply.github.com>
@jglick jglick merged commit ca5fddb into jenkinsci:master Sep 13, 2023
13 of 14 checks passed
@jglick jglick deleted the BufferedBuildListener-JENKINS-71970 branch September 13, 2023 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants