Skip to content
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.

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

Fix #453: use ProcessBuilder for output redirect #459

Merged
merged 1 commit into from
Apr 24, 2017

Conversation

sfnelson
Copy link
Contributor

Pull request #436 removes the call to exit that was added to address #171. This
caused a regression in testing and notify behaviour that was reported as #453,
where subprocesses would hang for 30 seconds after the subprocess completed.

This commit avoids the need for pump threads to copy data from subprocess
streams by using ProcessBuilder's existing Redirect functionality to either
write redirect stream output to files directly, or to inherit System.out and
System.err from the main process. This means that lein test etc now terminate
in a reasonable time again, and avoids the call to exit that was causing
problems for trampoline tasks.

Pull request emezeske#436 removes the call to exit that was added to address emezeske#171. This
caused a regression in testing and notify behaviour that was reported as emezeske#453,
where subprocesses would hang for 30 seconds after the subprocess completed.

This commit avoids the need for pump threads to copy data from subprocess
streams by using ProcessBuilder's existing Redirect functionality to either
write redirect stream output to files directly, or to inherit System.out and
System.err from the main process. This means that `lein test` etc now terminate
in a reasonable time again, and avoids the call to `exit` that was causing
problems for trampoline tasks.
@mneise mneise merged commit 4ba27e5 into emezeske:master Apr 24, 2017
@mneise
Copy link
Collaborator

mneise commented Apr 24, 2017

This looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants