RFR 9: 8132394 : (process) ProcessBuilder support for a pipeline of processes

[Paul Sandoz]

Hi Roger,

ProcessBuilder
—

 711      * The FileDescriptor is used as the standard input of the next Process
 712      * begin started.

s/begin/to be started
?


 714     static class RedirectPipeImpl extends Redirect {
 715         final FileDescriptor fd;
 716
 717         RedirectPipeImpl() {
 718             this.fd = new FileDescriptor();
 719         }
 720         public Type type() { return Type.PIPE; }
 721         public String toString() { return type().toString();}
 722         FileDescriptor getFd() { return fd; }
 723     }

Can you add @Override to appropriate methods?

Why do you need to use a FileDescriptor rather than an int? AFAICT FileDescriptor is used as a box to the underlying descriptor that is accessed via shared secrets.


1140      * All input and output streams between the intermediate processes are
1141      * not accessible, those redirects should be {@link Redirect#PIPE Redirect.PIPE}.

This was a little unclear to me until i read the documentation for throwing an ISE e.g. we need to talk about the process builder here.


1153      * @apiNote
1154      * For example to count the  unique imports for all the files in a file hierarchy:
1155      *
1156      * <pre>{@code
1157      * String directory = "/home/duke/src";
1158      * List<Process> processes = ProcessBuilder.startPipeline(
1159      *         new ProcessBuilder("find", directory,  "-type", "f"),
1160      *         new ProcessBuilder("xargs", "grep", "-h", "^import "),
1161      *         new ProcessBuilder("awk", "{print $2;}"),
1162      *         new ProcessBuilder("sort", "-u"));
1163      * Process last = processes.get(processes.size()-1);
1164      * try (InputStream is = last.getInputStream();
1165      *         Reader isr = new InputStreamReader(is);
1166      *         BufferedReader r = new BufferedReader(isr)) {
1167      *     long count = r.lines().count();
1168      * }
1169      * }</pre>

Perhaps you might want to qualify that example saying something like “on a UNIX compatible platform”.


1214             processes.forEach(p -> p.destroyForcibly());

If you feel in anyway inclined you can use a method reference here.


PipelineTest
—

Can you make the test method names more meaningful?

Paul.

> On 5 Nov 2015, at 22:56, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
> 
> Please review the new ProcessBuilder.startPipeline API, implementation, and tests.
> 
> The new method starts a Process for each ProcessBuilder, creating a pipeline of
> processes linked by their standard output and standard input streams.
> Each builder can use redirectErrorsream to coalesce error output with standard output.
> But otherwise standard error streams are not modified.
> 
> The API is the same as discussed on the earlier core-libs thread [1] and addresses
> the comments.
> 
> webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-pipeline-8132394/
> 
> javadoc of ProcessBuilder:  only startPipeline is new:
>   http://cr.openjdk.java.net/~rriggs/pipedoc/
> 
> Thanks, Roger
> 
> p.s. The PIPE_CHANNEL redirection proposed by Peter Levert is complementary
> and still has a bug or two to work out.
> 
> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-July/034634.html
> 
> 
> 
> 
>