[servlet-dev] Non-blocking IO and ServletOutputStream.close()

[Mark Thomas <markt@xxxxxxxxxx>]

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