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()

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.

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> 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
_______________________________________________
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