Skip to content

Commit fa7c376

Browse files
committed
Fix stdin handling and inheritIO default
- Change inheritIO default to false (breaking: set inheritIO(true) for interactive commands) - Remove redirectInput(DISCARD) - DISCARD is WRITE-only and crashes on Java 9+ - Close stdin by default in captured mode prevents hangs on read/timeout - Simplify timeout logic in waitForCompletion()
1 parent 45c91c3 commit fa7c376

2 files changed

Lines changed: 208 additions & 59 deletions

File tree

src/main/java/rife/bld/extension/ExecOperation.java

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@
4343
@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "intentional and documented")
4444
public class ExecOperation extends AbstractOperation<ExecOperation> {
4545

46-
public static final String COMMAND_NOT_VALID = "command values must not be null or empty";
46+
private static final String COMMAND_NOT_VALID = "command values must not be null or empty";
4747
private static final Logger logger = Logger.getLogger(ExecOperation.class.getName());
4848
private final List<String> args_ = new ArrayList<>();
4949
private final Map<String, String> env_ = new HashMap<>();
5050
private boolean failOnExit_ = true;
51-
private boolean inheritIO_;
51+
private boolean inheritIO_ = true;
5252
private int timeout_ = 30;
5353
private File workDir_;
5454

@@ -308,7 +308,7 @@ public ExecOperation fromProject(@NonNull BaseProject project) {
308308
* When {@code false}, stdout and stderr are merged and captured through the logger. This makes
309309
* output testable and keeps it in the build log, but breaks interactive prompts and ANSI formatting.
310310
* <p>
311-
* Default is {@code FALSE}
311+
* Default is {@code TRUE}
312312
*
313313
* @param inheritIO {@code true} to inherit I/O, {@code false} to capture output
314314
* @return this operation instance
@@ -330,7 +330,7 @@ public boolean isFailOnExit() {
330330
/**
331331
* Returns whether the child process inherits the I/O streams of the current JVM.
332332
*
333-
* @return {@code true} if I/O is inherited, {@code false} if output is captured
333+
* @return {@code true} if I/O is inherited (default), {@code false} if output is captured
334334
* @see #inheritIO(boolean)
335335
*/
336336
public boolean isInheritIO() {
@@ -472,8 +472,7 @@ public ExecOperation onUnix(@NonNull Collection<String> args) {
472472
* @see #onUnix(String...)
473473
* @see #isWindows() static method for complex conditional logic
474474
*/
475-
public ExecOperation onWindows
476-
(@NonNull String... args) {
475+
public ExecOperation onWindows(@NonNull String... args) {
477476
ObjectTools.requireAllNotEmpty(args, COMMAND_NOT_VALID);
478477
if (SystemTools.isWindows()) {
479478
return command(args);
@@ -585,6 +584,7 @@ private void cleanup(Process proc, Thread outputThread) {
585584
try {
586585
outputThread.join(1000);
587586
} catch (InterruptedException ignored) {
587+
Thread.currentThread().interrupt();
588588
}
589589
}
590590
}
@@ -610,6 +610,8 @@ private ProcessBuilder createProcessBuilder() {
610610
pb.inheritIO();
611611
} else {
612612
pb.redirectErrorStream(true);
613+
var devNull = SystemTools.isWindows() ? "NUL" : "/dev/null";
614+
pb.redirectInput(ProcessBuilder.Redirect.from(new File(devNull)));
613615
}
614616
return pb;
615617
}
@@ -642,13 +644,13 @@ private void readProcessOutput(Process proc) {
642644
try (var reader = new BufferedReader(
643645
new InputStreamReader(proc.getInputStream(), StandardCharsets.UTF_8))) {
644646
String line;
645-
while ((line = reader.readLine()) != null) {
647+
while (!Thread.currentThread().isInterrupted() && (line = reader.readLine()) != null) {
646648
if (logInfo) {
647649
logger.info(line);
648650
}
649651
}
650652
} catch (IOException e) {
651-
if (logSevere && proc.isAlive()) {
653+
if (logSevere && proc.isAlive() && !Thread.currentThread().isInterrupted()) {
652654
logger.log(Level.SEVERE, "Failed to read command output.", e);
653655
}
654656
}
@@ -688,16 +690,19 @@ private void waitForCompletion(Process proc) throws ExitStatusException {
688690
final var logSevere = logger.isLoggable(Level.SEVERE) && !silent();
689691

690692
try {
691-
boolean finished = proc.waitFor(timeout_, TimeUnit.SECONDS);
692-
if (!finished) {
693+
if (!proc.waitFor(timeout_, TimeUnit.SECONDS)) {
693694
proc.destroyForcibly();
694-
proc.waitFor(5, TimeUnit.SECONDS);
695-
if (logSevere) {
695+
if (!proc.waitFor(5, TimeUnit.SECONDS) && proc.isAlive()) {
696+
if (logSevere) {
697+
logger.severe("Process could not be killed after timeout.");
698+
}
699+
} else if (logSevere) {
696700
logger.severe("The command timed out after " + timeout_ + " seconds.");
697701
}
698702
throw new ExitStatusException(ExitStatusException.EXIT_FAILURE);
699703
}
700704
} catch (InterruptedException e) {
705+
Thread.currentThread().interrupt();
701706
if (logSevere) {
702707
logger.log(Level.SEVERE, "The command was interrupted.", e);
703708
}

0 commit comments

Comments
 (0)