[
https://issues.apache.org/jira/browse/GROOVY-11901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071112#comment-18071112
]
ASF GitHub Bot commented on GROOVY-11901:
-----------------------------------------
Copilot commented on code in PR #2431:
URL: https://github.com/apache/groovy/pull/2431#discussion_r3036107372
##########
src/main/java/org/codehaus/groovy/runtime/ProcessGroovyMethods.java:
##########
@@ -282,6 +286,102 @@ public static void waitForProcessOutput(Process self,
OutputStream output, Outpu
}
}
+ /**
+ * Executes the process and waits for it to complete, capturing
+ * the standard output, standard error, and exit code into a
+ * {@link ProcessResult}.
+ *
+ * <pre>
+ * def result = "ls -la".execute().waitForResult()
+ * if (result.ok) println result.out
+ * else System.err.println result.err
+ * </pre>
+ *
+ * @param self a Process
+ * @return a ProcessResult containing stdout, stderr, and exit code
+ * @throws InterruptedException if the current thread is interrupted.
+ * @since 6.0.0
+ */
+ public static ProcessResult waitForResult(Process self) throws
InterruptedException {
+ StringBuilder sout = new StringBuilder();
+ StringBuilder serr = new StringBuilder();
+ Thread tout = consumeProcessOutputStream(self, sout);
+ Thread terr = consumeProcessErrorStream(self, serr);
+ boolean interrupted = false;
+ try {
+ try { tout.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { terr.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { self.waitFor(); } catch (InterruptedException ignore) {
interrupted = true; }
+ closeStreams(self);
+ } finally {
+ if (interrupted) Thread.currentThread().interrupt();
+ }
+ return new ProcessResult(sout.toString(), serr.toString(),
self.exitValue());
+ }
+
+ /**
+ * Executes the process and waits for it to complete within the given
timeout,
+ * capturing the standard output, standard error, and exit code into a
+ * {@link ProcessResult}. If the process does not complete within the
timeout,
+ * it is forcibly destroyed.
+ *
+ * <pre>
+ * def result = "cmd".execute().waitForResult(30, TimeUnit.SECONDS)
+ * </pre>
+ *
+ * @param self a Process
+ * @param timeout the maximum time to wait
+ * @param unit the time unit of the timeout argument
+ * @return a ProcessResult containing stdout, stderr, and exit code
+ * @throws InterruptedException if the current thread is interrupted.
+ * @since 6.0.0
+ */
+ public static ProcessResult waitForResult(Process self, long timeout,
TimeUnit unit) throws InterruptedException {
+ StringBuilder sout = new StringBuilder();
+ StringBuilder serr = new StringBuilder();
+ Thread tout = consumeProcessOutputStream(self, sout);
+ Thread terr = consumeProcessErrorStream(self, serr);
+ boolean interrupted = false;
+ try {
+ try {
+ boolean finished = self.waitFor(timeout, unit);
+ if (!finished) {
+ self.destroyForcibly();
+ self.waitFor();
+ }
+ } catch (InterruptedException ignore) {
+ interrupted = true;
+ self.destroyForcibly();
+ }
+ try { tout.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { terr.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ closeStreams(self);
+ } finally {
+ if (interrupted) Thread.currentThread().interrupt();
+ }
+ return new ProcessResult(sout.toString(), serr.toString(),
self.exitValue());
+ }
Review Comment:
In `waitForResult(Process,long,TimeUnit)`, the `InterruptedException` catch
destroys the process but doesn't `waitFor()` it to actually terminate before
`exitValue()` is read. This can lead to `IllegalThreadStateException` and/or
background process leakage. After `destroyForcibly()`, ensure termination
(e.g., `self.waitFor()` in a `finally`/best-effort block) or rethrow the
interrupt so callers can handle it.
##########
src/test/groovy/groovy/execute/ExecuteTest.groovy:
##########
@@ -167,4 +178,225 @@ final class ExecuteTest {
assert out.toString().contains('hello')
assert process.exitValue() == 0
}
+
+ // -
> Renovate process handling methods
> ---------------------------------
>
> Key: GROOVY-11901
> URL: https://issues.apache.org/jira/browse/GROOVY-11901
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Minor
>
> Groovy's methods for process handling were created prior to the JDK having
> ProcessBuilder. We shouldn't replicate what it provides but there is plenty
> of scope to add a minimal Groovy-friendly facade in some cases.
> h3. Non-goals
> - Don't reimplement ProcessBuilder features
> - Don't break existing functionality
> - Leverage ProcessBuilder's fluent API when needed rather than proliferate
> extra extension methods
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
