brettwooldridge / NuProcess

Low-overhead, non-blocking I/O, external Process implementation for Java
Apache License 2.0
719 stars 84 forks source link

Sequential execution of Nuprocess is throwing IllegalStateException exception #28

Closed sachin-walia closed 9 years ago

sachin-walia commented 9 years ago

My use case is as follows:

  1. Scrape a webpage and extract all the images.
  2. One by one optimize those images by using linux binaries (optipng, turbojpeg etc).
  3. Above binaries are called from Nuprocess.

After first execution of external process I am getting following exception (subsequent execution failed):

java.lang.IllegalStateException: closeStdin() method has already been called.
    at com.zaxxer.nuprocess.internal.BasePosixProcess.wantWrite(BasePosixProcess.java:178)

Here is how the invocation of NuProcess looks like:

        NuProcessBuilder pb = new NuProcessBuilder(commandList);
        final ProcessHandler handler = new ProcessHandler(original.getAbsolutePath(), destination.getAbsolutePath(), jobId);
        pb.setProcessListener(handler);
        NuProcess process = pb.start();
        process.wantWrite();
        process.waitFor(0, TimeUnit.SECONDS);

Also my implementation of ProcessHandler

public class ProcessHandler  extends NuAbstractProcessHandler {
    private NuProcess nuProcess;
    private String originalFile;
    private String compressedFile;
    private String requestId;
    final Logger.ALogger LOGGER = Logger.of(this.getClass());

    public ProcessHandler(String originalFile, String compressedFile, String requestId){
        this.compressedFile = compressedFile;
        this.originalFile = originalFile;
        this.requestId = requestId;
    }

    @Override
    public void onStart(NuProcess nuProcess) {
        LOGGER.info("Request Id::" + requestId + "::Received encoding request for file::" + originalFile);
        this.nuProcess = nuProcess;
    }

    public void onExit(int exitCode){
        LOGGER.info("Request Id::" + requestId + "::Completed encoding of file::" + originalFile);
    }

    @Override
    public boolean onStdinReady(ByteBuffer buffer) {
        LOGGER.info("Request Id::" + requestId + "::Queueing encoding of file::" + originalFile);
        buffer.flip();
        return false; // false means we have nothing else to write at this time
    }

    @Override
    public void onStdout(ByteBuffer buffer) {
        if (buffer == null) {
            return;
        }
        byte[] bytes = new byte[buffer.remaining()];
        buffer.get(bytes);
        // We're done, so closing STDIN will cause the process to exit
        nuProcess.closeStdin();
    }
}
sachin-walia commented 9 years ago

Also my development OS is Mac OS X Yosemite in case if this helps.

brettwooldridge commented 9 years ago

Move process.wantWrite() to onStart() method. Simple race condition where onStdout() is being called quickly, before the process.wantWrite() has a chance to execute.

sachin-walia commented 9 years ago

Brett is there any overhead in moving process.wantWrite to onStart? Also do you anticipate any kind of race conditions for invoking large number of same system processes?

Thanks.

brettwooldridge commented 9 years ago

@sachinwalia2k8 There should be no overhead in moving the wantWrite() call. I do not anticiptate any race conditions invoking a large number of the same process, so if you encounter one please report it here as a bug.