From 7a7075012def2714f5db3e53dfed35aafc06ee47 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 19:58:41 -0500 Subject: [PATCH 1/4] Implement `afterDisconnect` in `SimpleCommandLauncher` --- .../hudson/test/SimpleCommandLauncher.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java index b8eea25e9..3f590aa08 100644 --- a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java +++ b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java @@ -27,16 +27,15 @@ import hudson.AbortException; import hudson.EnvVars; import hudson.Extension; +import hudson.Functions; import hudson.Util; import hudson.model.Descriptor; import hudson.model.Slave; import hudson.model.TaskListener; -import hudson.remoting.Channel; import hudson.slaves.ComputerLauncher; import hudson.slaves.SlaveComputer; import hudson.util.ProcessTree; import hudson.util.StreamCopyThread; -import java.io.IOException; import java.util.HashMap; import java.util.Map; import java.util.logging.Level; @@ -52,6 +51,8 @@ public class SimpleCommandLauncher extends ComputerLauncher { public final String cmd; private final Map env; + private transient Process proc; + private transient EnvVars cookie; @DataBoundConstructor // in case anyone needs to configRoundtrip such a node public SimpleCommandLauncher(String cmd) { @@ -72,29 +73,33 @@ public void launch(SlaveComputer computer, final TaskListener listener) { } listener.getLogger().println("$ " + cmd); ProcessBuilder pb = new ProcessBuilder(Util.tokenize(cmd)); - final EnvVars cookie = EnvVars.createCookie(); + cookie = EnvVars.createCookie(); pb.environment().putAll(cookie); if (env != null) { pb.environment().putAll(env); } - final Process proc = pb.start(); + proc = pb.start(); new StreamCopyThread("stderr copier for remote agent on " + computer.getDisplayName(), proc.getErrorStream(), listener.getLogger()).start(); - computer.setChannel(proc.getInputStream(), proc.getOutputStream(), listener.getLogger(), new Channel.Listener() { - @Override - public void onClosed(Channel channel, IOException cause) { - try { - ProcessTree.get().killAll(proc, cookie); - } catch (Exception x) { - LOGGER.log(Level.WARNING, null, x); - } - } - }); + computer.setChannel(proc.getInputStream(), proc.getOutputStream(), listener, null); LOGGER.log(Level.INFO, "agent launched for {0}", computer.getName()); } catch (Exception x) { LOGGER.log(Level.WARNING, null, x); } } + @Override + public void afterDisconnect(SlaveComputer computer, TaskListener listener) { + if (proc != null) { + try { + ProcessTree.get().killAll(proc, cookie); + } catch (Exception x) { + Functions.printStackTrace(x, listener.error("Failed to terminate process")); + } + proc = null; + cookie = null; + } + } + @Extension public static class DescriptorImpl extends Descriptor {} } From fb6d341da87c9b2f450c7f1c52263042e5e4250c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 20:41:10 -0500 Subject: [PATCH 2/4] Clearer logging --- .../java/org/jvnet/hudson/test/SimpleCommandLauncher.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java index 3f590aa08..9655e49a9 100644 --- a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java +++ b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java @@ -27,7 +27,6 @@ import hudson.AbortException; import hudson.EnvVars; import hudson.Extension; -import hudson.Functions; import hudson.Util; import hudson.model.Descriptor; import hudson.model.Slave; @@ -88,15 +87,18 @@ public void launch(SlaveComputer computer, final TaskListener listener) { } @Override - public void afterDisconnect(SlaveComputer computer, TaskListener listener) { + public synchronized void afterDisconnect(SlaveComputer computer, TaskListener listener) { if (proc != null) { try { ProcessTree.get().killAll(proc, cookie); + LOGGER.info(() -> "killed " + proc + " with " + cookie + " for " + computer.getName()); } catch (Exception x) { - Functions.printStackTrace(x, listener.error("Failed to terminate process")); + LOGGER.log(Level.WARNING, "failed to kill " + proc + " with " + cookie + " for " + computer.getName(), x); } proc = null; cookie = null; + } else { + LOGGER.info(() -> "no process for " + computer.getName()); } } From 027f924b6dae35bec72f7fe31114ed1539d58883 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 20:52:48 -0500 Subject: [PATCH 3/4] SpotBugs --- src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java index 9655e49a9..a036fc726 100644 --- a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java +++ b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java @@ -64,7 +64,7 @@ public SimpleCommandLauncher(String cmd) { } @Override - public void launch(SlaveComputer computer, final TaskListener listener) { + public synchronized void launch(SlaveComputer computer, final TaskListener listener) { try { Slave node = computer.getNode(); if (node == null) { From 86a2bc9d99d6ab83b0ef2057f89befab399641c2 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 18 Feb 2025 21:10:38 -0500 Subject: [PATCH 4/4] SpotBugs still --- .../java/org/jvnet/hudson/test/SimpleCommandLauncher.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java index a036fc726..64143a74d 100644 --- a/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java +++ b/src/main/java/org/jvnet/hudson/test/SimpleCommandLauncher.java @@ -24,6 +24,7 @@ package org.jvnet.hudson.test; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.EnvVars; import hudson.Extension; @@ -64,7 +65,7 @@ public SimpleCommandLauncher(String cmd) { } @Override - public synchronized void launch(SlaveComputer computer, final TaskListener listener) { + public void launch(SlaveComputer computer, final TaskListener listener) { try { Slave node = computer.getNode(); if (node == null) { @@ -86,6 +87,7 @@ public synchronized void launch(SlaveComputer computer, final TaskListener liste } } + @SuppressFBWarnings(value = "IS2_INCONSISTENT_SYNC", justification = "test code, close enough") @Override public synchronized void afterDisconnect(SlaveComputer computer, TaskListener listener) { if (proc != null) {