Skip to content

[JENKINS-17116] - When aborting a build, wait up to 2min for process termination #3414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oleg-nenashev merged 2 commits into from
Aug 31, 2018
Merged

[JENKINS-17116] - When aborting a build, wait up to 2min for process termination #3414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31cd4d3
[JENKINS-17116] - When aborting a job, wait up to 2 mins for process …
May 1, 2018
d1da38e
Allow customization of softkill time via property SoftKillWaitSeconds…
Aug 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jvnet.winp</groupId>
<artifactId>winp</artifactId>
<version>1.26</version>
<version>1.27</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci</groupId>
Expand Down
67 changes: 64 additions & 3 deletions core/src/main/java/hudson/util/ProcessTree.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ public final Iterator<OSProcess> iterator() {
*/
public abstract void killAll(Map<String, String> modelEnvVars) throws InterruptedException;

private final long softKillWaitSeconds = Integer.getInteger("SoftKillWaitSeconds", 2 * 60); // by default processes get at most 2 minutes to respond to SIGTERM (JENKINS-17116)

/**
* Convenience method that does {@link #killAll(Map)} and {@link OSProcess#killRecursively()}.
* This is necessary to reliably kill the process and its descendants, as some OS
Expand Down Expand Up @@ -501,6 +503,8 @@ public void killRecursively() throws InterruptedException {
return;

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();
}
Expand All @@ -512,10 +516,39 @@ public void kill() throws InterruptedException {
}

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();
Copy link
Member

@oleg-nenashev oleg-nenashev Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to determine whether the process got killed softly (e.g. by boolean return)? If yes, maybe it makes not sense to invoke other operations

Copy link
Author

@stephanreiter stephanreiter Jul 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside killSoftly we use isRunning to find out whether the process is around still. If not, we could skip the kill. But kill won't have any effect any way and I'd like to be cautious here and just add killSoftly as an extra step in addition to the old kill code.

killByKiller();
}

private void killSoftly() throws InterruptedException {
// send Ctrl+C to the process
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);
}
return;
}

// after that wait for it to cease to exist
long deadline = System.nanoTime() + softKillWaitSeconds * 1000000000;
int sleepTime = 10; // initially we sleep briefly, then sleep up to 1sec
do {
if (!p.isRunning()) {
break;
}

Thread.sleep(sleepTime);
sleepTime = Math.min(sleepTime * 2, 1000);
} while (System.nanoTime() < deadline);
}

@Override
public synchronized List<String> getArguments() {
if(args==null) {
Expand Down Expand Up @@ -718,12 +751,29 @@ protected final File getFile(String relativePath) {
* Tries to kill this process.
*/
public void kill() throws InterruptedException {
// after sending SIGTERM, wait for the process to cease to exist
long deadline = System.nanoTime() + softKillWaitSeconds * 1000000000;
kill(deadline);
}

private void kill(long deadline) throws InterruptedException {
if (getVeto() != null)
return;
try {
int pid = getPid();
LOGGER.fine("Killing pid="+pid);
UnixReflection.destroy(pid);
// after sending SIGTERM, wait for the process to cease to exist
int sleepTime = 10; // initially we sleep briefly, then sleep up to 1sec
File status = getFile("status");
do {
if (!status.exists()) {
break; // status is gone, process therefore as well
}

Thread.sleep(sleepTime);
sleepTime = Math.min(sleepTime * 2, 1000);
} while (System.nanoTime() < deadline);
} catch (IllegalAccessException e) {
// this is impossible
IllegalAccessError x = new IllegalAccessError();
Expand All @@ -740,11 +790,22 @@ public void kill() throws InterruptedException {
}

public void killRecursively() throws InterruptedException {
// after sending SIGTERM, wait for the processes to cease to exist until the deadline
long deadline = System.nanoTime() + softKillWaitSeconds * 1000000000;
killRecursively(deadline);
}

private void killRecursively(long deadline) throws InterruptedException {
// We kill individual processes of a tree, so handling vetoes inside #kill() is enough for UnixProcess es
LOGGER.fine("Recursively killing pid="+getPid());
for (OSProcess p : getChildren())
p.killRecursively();
kill();
for (OSProcess p : getChildren()) {
if (p instanceof UnixProcess) {
((UnixProcess)p).killRecursively(deadline);
} else {
p.killRecursively(); // should not happen, fallback to non-deadline version
}
}
kill(deadline);
}

/**
Expand Down