diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java index ca0f45b2ea93..58f2822db2c3 100644 --- a/core/src/main/java/hudson/slaves/SlaveComputer.java +++ b/core/src/main/java/hudson/slaves/SlaveComputer.java @@ -134,6 +134,11 @@ public class SlaveComputer extends Computer { */ private final TaskListener taskListener; + /** + * Flag to ensure afterDisconnect() is called only once per connection. + */ + private volatile boolean afterDisconnectCalled = false; + /** * Number of failed attempts to reconnect to this node @@ -650,6 +655,7 @@ public void onClosed(Channel c, IOException cause) { Functions.printStackTrace(cause, taskListener.error("Connection terminated")); } closeChannel(); + safeAfterDisconnect(); // changed from direct call to safe method to avoid multiple calls try { launcher.afterDisconnect(SlaveComputer.this, taskListener); } catch (Throwable t) { @@ -745,6 +751,9 @@ public void onClosed(Channel c, IOException cause) { this.absoluteRemoteFs = remoteFS; defaultCharset = Charset.forName(defaultCharsetName); + // Reset the flag for the new connection + afterDisconnectCalled = false; + synchronized (statusChangeLock) { statusChangeLock.notifyAll(); } @@ -769,6 +778,25 @@ public void onClosed(Channel c, IOException cause) { Jenkins.get().getQueue().scheduleMaintenance(); } + private void safeAfterDisconnect() { + if (!afterDisconnectCalled) { + synchronized (this) { + if (!afterDisconnectCalled) { + afterDisconnectCalled = true; + try { + launcher.afterDisconnect(SlaveComputer.this, taskListener); + } catch (Throwable t) { + LogRecord lr = new LogRecord(Level.SEVERE, + "Launcher {0}'s afterDisconnect method propagated an exception when {1}'s connection was closed: {2}"); + lr.setThrown(t); + lr.setParameters(new Object[]{launcher, SlaveComputer.this.getName(), t.getMessage()}); + logger.log(lr); + } + } + } + } + } + @Override public Channel getChannel() { return channel; @@ -825,7 +853,7 @@ public void run() { // (which could be typical) won't block UI thread. launcher.beforeDisconnect(SlaveComputer.this, taskListener); closeChannel(); - launcher.afterDisconnect(SlaveComputer.this, taskListener); + safeAfterDisconnect(); } }); } diff --git a/test/src/test/java/hudson/slaves/SlaveComputerTest.java b/test/src/test/java/hudson/slaves/SlaveComputerTest.java index 5278c0010a2c..0508ae371f48 100644 --- a/test/src/test/java/hudson/slaves/SlaveComputerTest.java +++ b/test/src/test/java/hudson/slaves/SlaveComputerTest.java @@ -45,6 +45,8 @@ import hudson.security.ACLContext; import java.io.IOError; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; + import jenkins.model.Jenkins; import net.sf.json.JSONNull; import net.sf.json.JSONObject; @@ -201,6 +203,45 @@ public void onOnline(Computer c, TaskListener listener) { } } } + @Test + @Issue("JENKINS-35272") + void afterDisconnectShouldBeCalledOnlyOnce() throws Exception { + DumbSlave node = j.createOnlineSlave(); + SlaveComputer computer = (SlaveComputer) node.getComputer(); + assertNotNull(computer); + + // Reset counter before test + AfterDisconnectListener.reset(); + + // Trigger disconnect twice + computer.disconnect(null); + computer.disconnect(null); + + // Wait until Jenkins finishes processing + j.waitUntilNoActivity(); + + assertEquals(1, AfterDisconnectListener.count.get(), + "afterDisconnect should be called exactly once"); + } + + @TestExtension + public static class AfterDisconnectListener extends ComputerListener { + + static final AtomicInteger count = new AtomicInteger(); + + static void reset() { + count.set(0); + } + + @Override + public void onOffline(Computer c, OfflineCause cause) { + if (c instanceof SlaveComputer) { + count.incrementAndGet(); + } + } + } + + /**