Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [servlet-dev] Non-blocking IO and ServletOutputStream.close() 
On 07/01/2023 06:57, Greg Wilkins wrote:

Mark,
Conceptually both close and complete could be thought of as write operations that need to wait until a true isReady() return before being called.
However, unlike a write operation, they cannot carry additional data to be queued, nor be called multiple times, so there is no unbounded data commitment if we let them be called whilst a write completion is still pending. In this interpretation, they are state changes rather than writes.
So since the spec has never said otherwise, i think we must use the second interpretation.
Thanks for the review Greg. There is a comment in the Tomcat code to the effect that complete()/dispatch() are not allowed while async IO is in progress. I need to do some research to see if I can find where that comment originated as that point is rather fundamental to this discussion. 

Mark
Furthermore, completion is orthogonal to output close. A response can be sent and the output closed before the entire request is read. Also computation can continue after both request and response bodies are complete and delaying a call to complete can create back pressure to prevent the next request being read (at least on some versions of HTTP).  So i do not think associating complete with output close is correct, nor do I think the container should ever complete for the application. > Thus i think that both close and complete can be called irrespective of isReady status. Both are state changes: close prevents any subsequent calls to write and will terminate response framing when any pending write completes; complete implies a close and will also arrange for the request cycle to be ended once any out standing calls to service, onDataAvailable or onWritePossible return.
I'm away from a proper computer at the moment, but will run through your scenario in more detail once i am back at a keyboard. 

Cheers
On Thu, 5 Jan 2023, 23:42 Mark Thomas, <markt@xxxxxxxxxx <mailto:markt@xxxxxxxxxx>> wrote: 

    Hi all,

    I am working on the changes required to Tomcat to implement the changes
    to ServletOutputStream.close() made as part of PR #485 [1].

    I have a question regarding the following scenario:

    1. HttpServletRequest.startAsync() is called to switch to async mode.

    2. ServletOutputStream.setWriteListener() is called to switch to
         non-blocking IO.

    3. There are then callbacks to WriteListener.onWritePossible()

    4. At some point during one of those callbacks
         ServletOutputStream.isReady() returns false and
         ServletOutputStream.close() is called.


    My question is who is responsible for calling AsyncContext.complete()
    and when does that call occur?


    If we assume that the application is expected to call
    AsyncContext.complete() then my expectation is that once the
    non-blocking write completes (and onWritePossible()) would normally be
    called, the application is required to call complete() at that
    point. In
    fact onWritePossible() needs to be called so the application knows when
    to call AsyncContext.complete().

    But that begs the question why allow ServletOutputStream.close() when
    ServletOutputStream.isReady() returns false? It would be much simpler
    not to allow this and the application can call
    ServletOutputStream.close() followed by AsyncContext.complete() on the
    next onWritePossible() call back. The end result is the same and the
    semantics are a lot simpler.


    If we assume that the container is expected to call
    AsyncContext.complete() once the blocking write has finished
    (instead of
    the onWritePossible() callback) then that is effectively saying that
    calling ServletOutputStream.close() is another way to terminate async
    processing.

    That seems to be quite a significant change to the specification.
    Currently, there are multiple places where it is stated an explicit
    call
    to complete() or one of the dispatch() methods is required to end async
    processing.

    This could result in multiple calls to complete() which will trigger
    ISEs. I think this could be addressed by having the container call
    complete() if the application hasn't already called
    complete()/dispatch(). I think that implies that complete()/dispatch()
    can also be called while a non-blocking write is in progress.

    Given that ServletOutputStream is Closeable I am also a little worried
    about unexpected / unintended consequences if we go this route.


    Whatever we do, I think some further clarification is required.

    I can see at least the following options:

    1. Change ServletOutputStream.close() so it can only be called after
    ServletOutputStream.isReady() has returned true (use the same language
    as for the write methods).

    2. Change ServletOutputStream.close() so the container writes out any
    remaining data and then calls AsyncContext.complete() (or calls
    complete() immediately if there is no data to write) unless the
    application has already done so.

    3. Something else I haven't thought of yet.


    Thoughts?


    Mark


    [1] https://github.com/jakartaee/servlet/pull/485
    <https://github.com/jakartaee/servlet/pull/485>
    _______________________________________________
    servlet-dev mailing list
    servlet-dev@xxxxxxxxxxx <mailto:servlet-dev@xxxxxxxxxxx>
    To unsubscribe from this list, visit
    https://www.eclipse.org/mailman/listinfo/servlet-dev
    <https://www.eclipse.org/mailman/listinfo/servlet-dev>


_______________________________________________
servlet-dev mailing list
servlet-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/servlet-dev

Back to the top