Skip to content

Commit e8e21bc

Browse files
committed
Fix JENKINS-35272: Safely prevent afterDisconnect() from being called twice
1 parent 057aa6a commit e8e21bc

File tree

1 file changed

+35
-2
lines changed

1 file changed

+35
-2
lines changed

core/src/main/java/hudson/slaves/SlaveComputer.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,18 @@ public class SlaveComputer extends Computer {
124124
*/
125125
private ComputerLauncher launcher;
126126

127+
/**
128+
* Lock used to synchronize the execution of {@link ComputerLauncher#afterDisconnect(SlaveComputer, TaskListener)}.
129+
* This ensures the disconnect logic is only executed once per connection cycle, and that
130+
* concurrent attempts to disconnect block until the first execution completes.
131+
* Blocking is critical to prevent file-lock race conditions during teardown on Windows.
132+
*/
133+
private final Object disconnectLock = new Object();
134+
/**
135+
* Guard flag protected by {@link #disconnectLock} to track if the launcher's afterDisconnect
136+
* has already been called for the current channel lifecycle.
137+
*/
138+
private boolean afterDisconnectCalled = false;
127139
/**
128140
* Perpetually writable log file.
129141
*/
@@ -651,7 +663,14 @@ public void onClosed(Channel c, IOException cause) {
651663
}
652664
closeChannel();
653665
try {
654-
launcher.afterDisconnect(SlaveComputer.this, taskListener);
666+
// Synchronize to prevent double execution (e.g., JENKINS-35272) while
667+
// forcing concurrent callers to wait for teardown to complete cleanly.
668+
synchronized (disconnectLock) {
669+
if (!afterDisconnectCalled) {
670+
afterDisconnectCalled = true;
671+
launcher.afterDisconnect(SlaveComputer.this, taskListener);
672+
}
673+
}
655674
} catch (Throwable t) {
656675
LogRecord lr = new LogRecord(Level.SEVERE,
657676
"Launcher {0}'s afterDisconnect method propagated an exception when {1}'s connection was closed: {2}");
@@ -742,6 +761,12 @@ public void onClosed(Channel c, IOException cause) {
742761
isUnix = _isUnix;
743762
numRetryAttempt = 0;
744763
this.channel = channel;
764+
765+
// Reset the disconnect guard for the new connection lifecycle
766+
synchronized (disconnectLock) {
767+
this.afterDisconnectCalled = false;
768+
}
769+
745770
this.absoluteRemoteFs = remoteFS;
746771
defaultCharset = Charset.forName(defaultCharsetName);
747772

@@ -823,7 +848,15 @@ public Future<?> disconnect(OfflineCause cause) {
823848
// (which could be typical) won't block UI thread.
824849
launcher.beforeDisconnect(SlaveComputer.this, taskListener);
825850
closeChannel();
826-
launcher.afterDisconnect(SlaveComputer.this, taskListener);
851+
852+
// Synchronize to prevent double execution (e.g., JENKINS-35272) while
853+
// forcing concurrent callers to wait for teardown to complete cleanly.
854+
synchronized (disconnectLock) {
855+
if (!afterDisconnectCalled) {
856+
afterDisconnectCalled = true;
857+
launcher.afterDisconnect(SlaveComputer.this, taskListener);
858+
}
859+
}
827860
});
828861
}
829862

0 commit comments

Comments
 (0)