From 4ec4a4282498295b96bccf9a10bd10924c19e568 Mon Sep 17 00:00:00 2001 From: Marat Radchenko Date: Mon, 23 Sep 2019 18:24:11 +0300 Subject: [PATCH] [JENKINS-17116][JENKINS-59152] Fix build abort not killing processes on Windows reliably Sending CTRL+C to the process was not a proper way to terminate processes. Many programs, most importantly cmd.exe, do not terminate if you send them CTRL+C. cmd.exe asks for Y/N confirmation when you send it a CTRL+C. Instead, always use TerminateProcess WinAPI call. This commit partially reverts changes from 31cd4d3d788a8dc28d0109e47971a0db84afaea0 Note that this commit doesn't fully fix JENKINS-59152 because there is still a race between process killing and REMOTE_TIMEOUT timer in DurableTaskStep. This problem needs to be fixed in workflow-durable-task-step-plugin. --- .../main/java/hudson/util/ProcessTree.java | 51 +++++++++---------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/hudson/util/ProcessTree.java b/core/src/main/java/hudson/util/ProcessTree.java index e8e48915c66f..545a3ea09266 100644 --- a/core/src/main/java/hudson/util/ProcessTree.java +++ b/core/src/main/java/hudson/util/ProcessTree.java @@ -514,45 +514,41 @@ public OSProcess getParent() { @Override public void killRecursively() throws InterruptedException { - if (getVeto() != null) - return; + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(softKillWaitSeconds); + killRecursively(deadline); + } - LOGGER.log(FINER, "Killing recursively {0}", getPid()); - // Firstly try to kill the root process gracefully, then do a forcekill if it does not help (algorithm is described in JENKINS-17116) - killSoftly(); - p.killRecursively(); - killByKiller(); + private void killRecursively(long deadline) throws InterruptedException { + LOGGER.fine("Recursively killing pid="+getPid()); + for (OSProcess p : getChildren()) { + if (p instanceof WindowsOSProcess) { + ((WindowsOSProcess)p).killRecursively(deadline); + } else { + p.killRecursively(); // should not happen, fallback to non-deadline version + } + } + kill(deadline); } @Override public void kill() throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(softKillWaitSeconds); + kill(deadline); + } + + private void kill(long deadline) throws InterruptedException { if (getVeto() != null) { return; } - - LOGGER.log(FINER, "Killing {0}", getPid()); - // Firstly try to kill it gracefully, then do a forcekill if it does not help (algorithm is described in JENKINS-17116) - killSoftly(); - p.kill(); - killByKiller(); - } - - private void killSoftly() throws InterruptedException { - // send Ctrl+C to the process + final int pid = getPid(); + LOGGER.log(FINER, "Killing {0}", pid); try { - if (!p.sendCtrlC()) { - return; - } - } - catch (WinpException e) { - if (LOGGER.isLoggable(Level.FINE)) { - LOGGER.log(Level.FINE, "Failed to send CTRL+C to pid=" + getPid(), e); - } + p.kill(); + } catch (WinpException e) { + LOGGER.log(Level.INFO, "Failed to terminate pid=" + pid, e); return; } - // after that wait for it to cease to exist - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(softKillWaitSeconds); int sleepTime = 10; // initially we sleep briefly, then sleep up to 1sec do { if (!p.isRunning()) { @@ -562,6 +558,7 @@ private void killSoftly() throws InterruptedException { Thread.sleep(sleepTime); sleepTime = Math.min(sleepTime * 2, 1000); } while (System.nanoTime() < deadline); + killByKiller(); } @Override