Skip to content

Commit

Permalink
Revert "Allow disabling process killing on interruption (#3293)"
Browse files Browse the repository at this point in the history
This reverts commit e7fbce3.
  • Loading branch information
daniel-beck committed Mar 10, 2018
1 parent 20bc2e8 commit fe38050
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 52 deletions.
43 changes: 8 additions & 35 deletions core/src/main/java/hudson/Launcher.java
Expand Up @@ -181,12 +181,6 @@ public final class ProcStarter {
*/
protected boolean reverseStdin, reverseStdout, reverseStderr;

/**
* True to prevent killing the launched process when it is interrupted
* @since TODO
*/
protected boolean dontKillWhenInterrupted;

/**
* Passes a white-space separated single-string command (like "cat abc def") and parse them
* as a command argument. This method also handles quotes.
Expand Down Expand Up @@ -447,22 +441,6 @@ public ProcStarter writeStdin() {
return this;
}

/**
* Indicates that the launched process should not be killed when interrupted.
* It allows detecting the interruption on caller's side and do custom (cleanup) action while
* the launched process is still running.
*
* <p>
* Note that the process can (and should) be killed
* via {@link Proc#kill()} when custom action is done.
*
* @return {@code this}
* @since TODO
*/
public ProcStarter dontKillWhenInterrupted() {
this.dontKillWhenInterrupted = true;
return this;
}

/**
* Starts the new process as configured.
Expand Down Expand Up @@ -948,7 +926,7 @@ public Proc launch(ProcStarter ps) throws IOException {
ps.reverseStdin ?LocalProc.SELFPUMP_INPUT:ps.stdin,
ps.reverseStdout?LocalProc.SELFPUMP_OUTPUT:ps.stdout,
ps.reverseStderr?LocalProc.SELFPUMP_OUTPUT:ps.stderr,
toFile(ps.pwd), ps.dontKillWhenInterrupted);
toFile(ps.pwd));
}

private File toFile(FilePath f) {
Expand Down Expand Up @@ -1071,8 +1049,7 @@ public Proc launch(ProcStarter ps) throws IOException {
final String workDir = psPwd==null ? null : psPwd.getRemote();

try {
return new ProcImpl(getChannel().call(new RemoteLaunchCallable(ps.commands, ps.masks, ps.envs, in, ps.reverseStdin,
out, ps.reverseStdout, err, ps.reverseStderr, ps.quiet, workDir, listener, ps.dontKillWhenInterrupted)));
return new ProcImpl(getChannel().call(new RemoteLaunchCallable(ps.commands, ps.masks, ps.envs, in, ps.reverseStdin, out, ps.reverseStdout, err, ps.reverseStderr, ps.quiet, workDir, listener)));
} catch (InterruptedException e) {
throw (IOException)new InterruptedIOException().initCause(e);
}
Expand Down Expand Up @@ -1290,14 +1267,12 @@ private static class RemoteLaunchCallable extends MasterToSlaveCallable<RemotePr
private final @Nonnull TaskListener listener;
private final boolean reverseStdin, reverseStdout, reverseStderr;
private final boolean quiet;
private final boolean dontKillWhenInterrupted;

RemoteLaunchCallable(@Nonnull List<String> cmd, @CheckForNull boolean[] masks, @CheckForNull String[] env,
@CheckForNull InputStream in, boolean reverseStdin,
@CheckForNull OutputStream out, boolean reverseStdout,
@CheckForNull OutputStream err, boolean reverseStderr,
boolean quiet, @CheckForNull String workDir,
@Nonnull TaskListener listener, boolean dontKillWhenInterrupted) {

RemoteLaunchCallable(@Nonnull List<String> cmd, @CheckForNull boolean[] masks, @CheckForNull String[] env,
@CheckForNull InputStream in, boolean reverseStdin,
@CheckForNull OutputStream out, boolean reverseStdout,
@CheckForNull OutputStream err, boolean reverseStderr,
boolean quiet, @CheckForNull String workDir, @Nonnull TaskListener listener) {
this.cmd = new ArrayList<>(cmd);
this.masks = masks;
this.env = env;
Expand All @@ -1310,7 +1285,6 @@ private static class RemoteLaunchCallable extends MasterToSlaveCallable<RemotePr
this.reverseStdout = reverseStdout;
this.reverseStderr = reverseStderr;
this.quiet = quiet;
this.dontKillWhenInterrupted = dontKillWhenInterrupted;
}

public RemoteProcess call() throws IOException {
Expand All @@ -1321,7 +1295,6 @@ public RemoteProcess call() throws IOException {
if (reverseStdin) ps.writeStdin();
if (reverseStdout) ps.readStdout();
if (reverseStderr) ps.readStderr();
if (dontKillWhenInterrupted) ps.dontKillWhenInterrupted();

final Proc p = ps.start();

Expand Down
23 changes: 6 additions & 17 deletions core/src/main/java/hudson/Proc.java
Expand Up @@ -64,12 +64,6 @@
public abstract class Proc {
protected Proc() {}

/**
* Indicates that the process should not be killed on interruption.
* @since TODO
*/
protected boolean dontKillWhenInterrupted;

/**
* Checks if the process is still alive.
*/
Expand Down Expand Up @@ -213,17 +207,17 @@ public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out) thro
}

public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out, File workDir) throws IOException {
this(cmd,env,in,out,null,workDir, true);
this(cmd,env,in,out,null,workDir);
}

/**
* @param err
* null to redirect stderr to stdout.
*/
public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir, boolean dontKillWhenInterrupted) throws IOException {
public LocalProc(String[] cmd,String[] env,InputStream in,OutputStream out,OutputStream err,File workDir) throws IOException {
this( calcName(cmd),
stderr(environment(new ProcessBuilder(cmd),env).directory(workDir), err==null || err== SELFPUMP_OUTPUT),
in, out, err, dontKillWhenInterrupted);
in, out, err );
}

private static ProcessBuilder stderr(ProcessBuilder pb, boolean redirectError) {
Expand All @@ -243,11 +237,10 @@ private static ProcessBuilder environment(ProcessBuilder pb, String[] env) {
return pb;
}

private LocalProc( String name, ProcessBuilder procBuilder, InputStream in, OutputStream out, OutputStream err, boolean dontKillWhenInterrupted ) throws IOException {
private LocalProc( String name, ProcessBuilder procBuilder, InputStream in, OutputStream out, OutputStream err ) throws IOException {
Logger.getLogger(Proc.class.getName()).log(Level.FINE, "Running: {0}", name);
this.name = name;
this.out = out;
this.dontKillWhenInterrupted = dontKillWhenInterrupted;
this.cookie = EnvVars.createCookie();
procBuilder.environment().putAll(cookie);
if (procBuilder.directory() != null && !procBuilder.directory().exists()) {
Expand Down Expand Up @@ -361,9 +354,7 @@ public int join() throws InterruptedException, IOException {
return r;
} catch (InterruptedException e) {
// aborting. kill the process
if (!dontKillWhenInterrupted) {
destroy();
}
destroy();
throw e;
} finally {
t.setName(oldName);
Expand Down Expand Up @@ -471,9 +462,7 @@ public int join() throws IOException, InterruptedException {
return process.get();
} catch (InterruptedException e) {
LOGGER.log(Level.FINE, String.format("Join operation has been interrupted for the process %s. Killing the process", this), e);
if (!dontKillWhenInterrupted) {
kill();
}
kill();
throw e;
} catch (ExecutionException e) {
if(e.getCause() instanceof IOException)
Expand Down

0 comments on commit fe38050

Please sign in to comment.