From e4fe5042e4f591b1e09ae4ec78354a8a5cf57370 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 8 Nov 2023 08:39:28 -0500 Subject: [PATCH] Temporary memory leak in `FutureImpl.executors` (#8640) * Temporary memory leak in `FutureImpl.executors` * https://github.com/jenkinsci/jenkins-test-harness/pull/669 released * Skip new test on Windows https://github.com/jenkinsci/jenkins/pull/8640#discussion_r1373797374 --- .../java/hudson/model/queue/FutureImpl.java | 4 ++++ .../hudson/model/queue/WorkUnitContext.java | 1 + .../test/java/hudson/model/ComputerTest.java | 24 +++++++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/core/src/main/java/hudson/model/queue/FutureImpl.java b/core/src/main/java/hudson/model/queue/FutureImpl.java index f6bba013f2..d57453ece5 100644 --- a/core/src/main/java/hudson/model/queue/FutureImpl.java +++ b/core/src/main/java/hudson/model/queue/FutureImpl.java @@ -95,4 +95,8 @@ public final class FutureImpl extends AsyncFutureImpl implements Que synchronized void addExecutor(@NonNull Executor executor) { this.executors.add(executor); } + + synchronized void finished() { + executors.clear(); + } } diff --git a/core/src/main/java/hudson/model/queue/WorkUnitContext.java b/core/src/main/java/hudson/model/queue/WorkUnitContext.java index 31829ba7dd..83320ab5d4 100644 --- a/core/src/main/java/hudson/model/queue/WorkUnitContext.java +++ b/core/src/main/java/hudson/model/queue/WorkUnitContext.java @@ -190,6 +190,7 @@ public final class WorkUnitContext { } } } + future.finished(); } } } diff --git a/test/src/test/java/hudson/model/ComputerTest.java b/test/src/test/java/hudson/model/ComputerTest.java index b0ce7a7546..a3392ee448 100644 --- a/test/src/test/java/hudson/model/ComputerTest.java +++ b/test/src/test/java/hudson/model/ComputerTest.java @@ -31,6 +31,7 @@ import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isA; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -39,12 +40,16 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeThat; import hudson.ExtensionList; +import hudson.Functions; import hudson.diagnosis.OldDataMonitor; +import hudson.remoting.Channel; import hudson.slaves.DumbSlave; import hudson.slaves.OfflineCause; import java.io.File; +import java.lang.ref.WeakReference; import java.net.HttpURLConnection; import java.nio.charset.StandardCharsets; import java.util.Map; @@ -66,6 +71,7 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; import org.jvnet.hudson.test.LoggerRule; +import org.jvnet.hudson.test.MemoryAssert; import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.MockFolder; import org.jvnet.hudson.test.SmokeTest; @@ -263,4 +269,22 @@ public class ComputerTest { j.assertBuildStatus(Result.FAILURE, j.waitForCompletion(b)); } + @Test + public void computersCollected() throws Exception { + assumeThat("Seems to crash the test JVM at least in CI", Functions.isWindows(), is(false)); + DumbSlave agent = j.createOnlineSlave(); + FreeStyleProject p = j.createFreeStyleProject(); + p.setAssignedNode(agent); + j.buildAndAssertSuccess(p); + Computer computer = agent.toComputer(); + WeakReference computerRef = new WeakReference<>(computer); + WeakReference channelRef = new WeakReference<>((Channel) computer.getChannel()); + computer.disconnect(null); + computer = null; + j.jenkins.removeNode(agent); + agent = null; + MemoryAssert.assertGC(computerRef, false); + MemoryAssert.assertGC(channelRef, false); + } + }