Bad timeout semantic in Response.suspend method

[arinban]

Most versions of Response.suspend method have timeout argument. It is really important and necessary if making the answer needs considerable time (that is the main reason of using "suspend" method instead of simple returning the answer).

Alexey Stashok told me that the semantics of this timeout is very simple: it is the time since the moment of call "suspend" method. For example, if I specifies timeout 30 seconds, and the response will not be resumed in 30 seconds after this moment, then the response will be stopped, connection with the client will be interrupted and my TimeoutHandler will be invoked (if specified).

But it is very bad logic. "Suspend" is used in situations, when the returned data can be very large and creating them requires long time. For example, it is a large image or ZIP-archive - results of image processing or recognition. What timeout should I set? If I set relative little (30 seconds), but the result of the request is 100-MB, there is a risk that this time will be not enough to pass all data (via WriteHandler.onWritePossible) to the client. Sometimes, depending on internet speed, downloading large data and even forming them can require 1-2 hours or more. If I set very large timeout (12 hours), it loses its sense: problems while forming data or with internet will "eat" resources during many hours.

I offer to change logic to more typical behavior. Let timeout be calculated from LAST ATTEMPT to sent any data to client. For example, if we have timeout 30 seconds, and the server cannot sent to client anything during first 30 seconds, timeout will occur. But if the server will sent something via NIOOutputStream (like the common header of data), timeout will reset again to 0. If the server will send every portion of data every 1, 5 or even 10 seconds, the timeout will never occur. And only in a case of serious problem, when the server (my method onWritePossible) cannot send anything during more than 30 seconds - that probably signals about serious problems with internet connection or in algorithm forming the data - only in this case timeout will occur and my TimeoutHandler will be invoked.

I believe it can be fixed easily and will make the behavior more adequate.

Environment

Java 8

Affected Versions

[2.3.22]

[arinban]

• Issue Imported From: https://github.com/javaee/grizzly/issues/1869
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To:@rlubke
• Original Issue Closed By:@glassfishrobot

[arinban]

@glassfishrobot Commented Reported by daniel_alievsky

[arinban]

@glassfishrobot Commented @rlubke said: The current behavior is inline with the Servlet specification.

You can achieve similar functionality as what you describe by calling SuspendContext.setTimeout() after an I/O operation succeeds.

[arinban]

@glassfishrobot Commented daniel_alievsky said: Thank you. Can you clarify what exactly should I call? Below is an example:

configuration.addHttpHandler(
            new HttpHandler() {
public void service(Request request, final Response response) throws Exception {
    final SimpleDateFormat format = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
    System.out.println("Processing " + request.getRequestURI() + " - " + request.getRequestURL());
    System.out.println("  method: " + request.getMethod());
    System.out.println("  port: " + request.getServerPort());
    System.out.println("  remote port: " + request.getRemotePort());
    System.out.println("  path info: " + request.getPathInfo());
    System.out.println("  context: " + request.getContextPath());
    System.out.println("  encoding: " + request.getCharacterEncoding());
    System.out.println("  query: " + request.getQueryString());
    for (String name : request.getParameterNames()) {
        System.out.printf("%s: %s%n", name, request.getParameter(name));
    }
    System.out.println("  headers:");
    for (String headerName : request.getHeaderNames()) {
        for (String headerValue : request.getHeaders(headerName)) {
            System.out.printf("    %s=%s%n", headerName, headerValue);
        }
    }
    System.out.println("input buffer: " + request.getInputBuffer().getBuffer());
    final String result = format.format(new Date(System.currentTimeMillis()))
        + " at " + request.getRequestURI();
    response.setContentType("text/plain");
    final NIOOutputStream outputStream = response.getNIOOutputStream();
    response.suspend(10, TimeUnit.SECONDS, null, new TimeoutHandler() {
        @Override
        public boolean onTimeout(Response response) {
            //It is timeout from the very beginning of the request: must be large for large responses
            System.out.println("TIMEOUT!!!");
            try {
outputStream.close();
            } catch (IOException ignored) {
            }
            request.getRequest().getConnection().closeSilently();
            return true;
        }
    });
    new Thread() {
        @Override
        public void run() {
            for (long t = System.currentTimeMillis(); System.currentTimeMillis() - t < 5000; ) {
            }
            outputStream.notifyCanWrite(
new WriteHandler() {
    @Override
    public void onWritePossible() throws Exception {
        final byte[] bytes = result.getBytes();
        final int firstPortion = Math.min(25, bytes.length);
        response.setContentLength(result.length());
        //TODO!! reset timeout
        outputStream.write(Arrays.copyOfRange(bytes, 0, firstPortion));
        outputStream.flush();
        System.out.printf("Sending %d bytes...%n", firstPortion);
        for (long t = System.currentTimeMillis();
             System.currentTimeMillis() - t < 20000; ) {
        }
        System.out.printf("Sending %d bytes...%n", bytes.length - firstPortion);
        //TODO!! reset timeout
        outputStream.write(Arrays.copyOfRange(bytes, firstPortion, bytes.length));
        outputStream.close();
        if (response.isSuspended()) {
            System.out.println("Resumed");
            response.resume();
        } else {
            System.out.println("Finished");
            response.finish();
        }
    }

    @Override
    public void onError(Throwable t) {
        System.out.println("ERROR!");
        t.printStackTrace();
    }
});
        }
    }.start();
}
            },
            "/");
        server.start();
        System.out.println("Press ENTER to stop the server...");
        System.in.read();
        server.shutdown();

(Full text is available at https://github.com/simagis/simagis-tests/blob/master/grizzly-tests/src/test/java/com/simagis/pyramid/grizzly/tests/GrizzlyServerTest.java ) What should I write instead of //TODO!! reset timeout ?

[arinban]

@glassfishrobot Commented @rlubke said: Obtain a final reference to the SuspendContext for this request/response by calling Response.getSuspendContext(). Replace your //TODO comments with calls to SuspendContext.setTimeout(long, TimeUnit).

[arinban]

@glassfishrobot Commented daniel_alievsky said: Thank you, it works fine.

[arinban]

@glassfishrobot Commented This issue was imported from java.net JIRA GRIZZLY-1869

[arinban]

@glassfishrobot Commented Marked as won't fix on Wednesday, September 28th 2016, 2:56:11 pm