paulk-asert opened a new pull request, #2935:
URL: https://github.com/apache/groovy/pull/2935
executeCommand started the two stream-reader threads and then waited for the
process, but nothing waited for the threads:
proc.consumeProcessOutput(sout, serr) // starts the readers
int exitCode = proc.waitFor() // waits for the process only
if (sout.size()) print sout // may run before they append
consumeProcessOutput starts the readers and returns; waitFor waits for the
process to exit, which can happen while output is still sitting in the pipe
unread. The command could therefore print nothing for a shell command that
produced output. waitForProcessOutput joins both readers and then waits for the
process, which is the guarantee this code needs; its javadoc makes the
distinction explicit. exitValue is safe after it, since the process has
finished.
This is the only occurrence of the shape in product code. The two in
`WorkingWithIOSpecTest` are the documented drain-to-avoid-blocking idiom, where
the output is discarded, so they are correct as they stand.
The failure seen in CI is `ExtraConsoleCommandsTest.bangRunsAPortableEcho`
asserting on an empty buffer, intermittently and on a different JDK each time.
That is consistent with this race, but it did not reproduce locally, so the fix
is made on the strength of the code being wrong rather than on a demonstrated
repro.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
