Skip to content

Commit 9c60979

Browse files
committed
Revert "Fix JENKINS-35272: Prevent afterDisconnect() from being called twice"
Causes Windows test failures in UnsupportedRemotingAgentTest Fixes #26335 Reverts pull request: * #26188 This reverts commit 799d362.
1 parent 1802fbd commit 9c60979

File tree

2 files changed

+3
-71
lines changed

2 files changed

+3
-71
lines changed

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

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
import java.util.Map;
7676
import java.util.TreeMap;
7777
import java.util.concurrent.Future;
78-
import java.util.concurrent.atomic.AtomicBoolean;
7978
import java.util.logging.Handler;
8079
import java.util.logging.Level;
8180
import java.util.logging.LogRecord;
@@ -151,13 +150,6 @@ public class SlaveComputer extends Computer {
151150

152151
private Object constructed = new Object();
153152

154-
/**
155-
* Flag to ensure afterDisconnect() is only called once per disconnect cycle.
156-
* Reset when a new connection is established.
157-
* Thread-safe to handle concurrent disconnect scenarios.
158-
*/
159-
private final AtomicBoolean afterDisconnectCalled = new AtomicBoolean(false);
160-
161153
private transient volatile String absoluteRemoteFs;
162154

163155
public SlaveComputer(Slave slave) {
@@ -640,9 +632,6 @@ public void setChannel(@NonNull Channel channel,
640632
if (this.channel != null)
641633
throw new IllegalStateException("Already connected");
642634

643-
// Reset the flag for the new connection
644-
afterDisconnectCalled.set(false);
645-
646635
final TaskListener taskListener = launchLog != null ? new StreamTaskListener(launchLog) : TaskListener.NULL;
647636
PrintStream log = taskListener.getLogger();
648637

@@ -662,9 +651,7 @@ public void onClosed(Channel c, IOException cause) {
662651
}
663652
closeChannel();
664653
try {
665-
if (afterDisconnectCalled.compareAndSet(false, true)) {
666-
launcher.afterDisconnect(SlaveComputer.this, taskListener);
667-
}
654+
launcher.afterDisconnect(SlaveComputer.this, taskListener);
668655
} catch (Throwable t) {
669656
LogRecord lr = new LogRecord(Level.SEVERE,
670657
"Launcher {0}'s afterDisconnect method propagated an exception when {1}'s connection was closed: {2}");
@@ -836,10 +823,7 @@ public Future<?> disconnect(OfflineCause cause) {
836823
// (which could be typical) won't block UI thread.
837824
launcher.beforeDisconnect(SlaveComputer.this, taskListener);
838825
closeChannel();
839-
// Only call afterDisconnect if it hasn't been called yet
840-
if (afterDisconnectCalled.compareAndSet(false, true)) {
841-
launcher.afterDisconnect(SlaveComputer.this, taskListener);
842-
}
826+
launcher.afterDisconnect(SlaveComputer.this, taskListener);
843827
});
844828
}
845829

test/src/test/java/hudson/model/ComputerTest.java

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import java.nio.charset.StandardCharsets;
5555
import java.time.Duration;
5656
import java.util.Map;
57-
import java.util.concurrent.Future;
5857
import java.util.concurrent.TimeUnit;
5958
import java.util.logging.Level;
6059
import jenkins.model.Jenkins;
@@ -193,8 +192,7 @@ private void verifyOfflineCause(Computer computer) throws Exception {
193192
@Test
194193
void addAction() throws Exception {
195194
Computer c = j.createSlave().toComputer();
196-
class A extends InvisibleAction {
197-
}
195+
class A extends InvisibleAction {}
198196

199197
assertEquals(0, c.getActions(A.class).size());
200198
c.addAction(new A());
@@ -369,54 +367,4 @@ public void isConnectedTest() throws Exception {
369367
assertThat(computer.isConnected(), is(false));
370368
assertThat(computer.isOffline(), is(true));
371369
}
372-
373-
@Issue("JENKINS-17204")
374-
@Test
375-
void testMultipleDisconnectCallsHandledProperly() throws Exception {
376-
DumbSlave agent = j.createOnlineSlave();
377-
Computer computer = agent.toComputer();
378-
379-
// Sanity check
380-
assertTrue(computer.isOnline(), "Agent should be online initially");
381-
382-
// Simulate multiple concurrent disconnect calls
383-
Future<?> f1 = computer.disconnect(
384-
new OfflineCause.UserCause(User.getUnknown(), "disconnect 1"));
385-
Future<?> f2 = computer.disconnect(
386-
new OfflineCause.UserCause(User.getUnknown(), "disconnect 2"));
387-
Future<?> f3 = computer.disconnect(
388-
new OfflineCause.UserCause(User.getUnknown(), "disconnect 3"));
389-
390-
// Wait for all disconnects to complete
391-
f1.get();
392-
f2.get();
393-
f3.get();
394-
395-
// Agent must be cleanly disconnected
396-
assertFalse(computer.isOnline(), "Computer should be offline");
397-
assertFalse(computer.isConnected(), "Computer should not be connected");
398-
}
399-
400-
@Issue("JENKINS-17204")
401-
@Test
402-
void testAfterDisconnectFlagResetOnReconnection() throws Exception {
403-
DumbSlave agent = j.createOnlineSlave();
404-
Computer computer = agent.toComputer();
405-
406-
// First disconnect
407-
computer.disconnect(
408-
new OfflineCause.UserCause(User.getUnknown(), "first disconnect")).get();
409-
assertFalse(computer.isOnline(), "Computer should be offline after first disconnect");
410-
411-
// Reconnect
412-
computer.connect(false);
413-
await()
414-
.atMost(Duration.ofSeconds(30))
415-
.until(computer::isOnline);
416-
417-
// Disconnect again — flag must be reset
418-
computer.disconnect(
419-
new OfflineCause.UserCause(User.getUnknown(), "second disconnect")).get();
420-
assertFalse(computer.isOnline(), "Computer should be offline after second disconnect");
421-
}
422370
}

0 commit comments

Comments
 (0)