-
Notifications
You must be signed in to change notification settings - Fork 30
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
Added InputStreamCollector #50
Conversation
Multithreaded way to collect inputStreams
|
||
private volatile IOException exception; | ||
|
||
public InputStreamCollector(@Nonnull InputStream is, @Nullable PrintStream ps, @Nullable Charset cs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we assume that things are @Nonnull
by default, similarly to how Guava and Checker Framework approach things, so I'd vote to have this instance of @Nonnull
removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm OK with that.
throw stdInputThread.getException(); | ||
} else if (stdErrorThread.getException() != null) { | ||
throw stdErrorThread.getException(); | ||
} else if (exitValue != EXIT_VALUE_SUCCESS) { | ||
throw new RuntimeException("'" + cmd + "' exited with " + exitValue); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noted this issue: Maybe it's not the best idea to re-throw exceptions from another thread without wrapping them, since the stacktrace of the exception won't match to the thread that throws it :-/
- Wrapped exceptions from other threads - Removed unnecessary @nonnull
Thanks very much! |
Multithreaded way to collect inputStreams of started process in order to prevent deadlocks when buffer for StdErr is full while parent process still tries to read from StdOut.