Temporary memory leak in FutureImpl.executors (#8640)

* Temporary memory leak in `FutureImpl.executors` * jenkinsci/jenkins-test-harness#669 released * Skip new test on Windows #8640 (comment)
jenkinsci · Nov 8, 2023 · e4fe504 · e4fe504
1 parent 942f932
commit e4fe504
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 0 deletions.
diff --git 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 synchronized void setAsCancelled() {
     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
@@ -190,6 +190,7 @@ public void synchronizeEnd(Executor e, Queue.Executable executable, Throwable pr
                         }
                     }
                 }
+                future.finished();
             }
         }
     }

diff --git 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.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.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.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 void testTerminatedNodeAjaxExecutorsDoesNotShowTrace() throws Exception {
         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<Computer> computerRef = new WeakReference<>(computer);
+        WeakReference<Channel> 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);
+    }
+
 }