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,

Also relevant is the 2019 discussion in https://github.com/jakartaee/servlet/issues/273 where I was again making the case that complete was a write and should wait for an isReady()==true before being legal to be called.

You can see that until that point, Jetty was implemented to treat a close or complete with a write pending as an error.   After that discussion and in response to user reported issues, jetty changed in Dec 2019 to follow undertow's model of allowing close and complete to be called with async operations pending.   These changes were also closely associated with a rethink/reimplement of sendError, which equally became a non-blocking state change method rather than a potentially blocking invocation of an error page dispatch.

It was a difficult reimplementation.... so perhaps a touch of I-don't-want-to-change-it-back bias now makes me think that the state change model is not so bad.

cheers


On Tue, 10 Jan 2023 at 03:01, Mark Thomas <markt@xxxxxxxxxx> wrote:
On 09/01/2023 09:04, Mark Thomas wrote:
> 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.

The comment in the Tomcat code is [1]. It was added by Filip when he
wrote the original test [2]. That is consistent with when the Servlet
3.1 API was being discussed as part of JSR 340.

I went back through the JSR 340 EG discussions to see if I could find
anything that could have led to Filip adding that comment.

The initial draft for async IO included a write() followed immediately
by a complete() [3].

When I asked what looks like the same question almost a decade ago, the
consensus was that calling write() or complete() with a write in
progress should trigger an exception [4].

The topic was raised again in Servlet 4.0 / JSR 369 [5] and the
conclusion was the same - calling complete() with a write in progress
should trigger an exception.

I haven't found any further discussions since until this one.

I don't think any of the conclusions from those discussions made it into
the spec document or the Javadoc.

The TCK includes a test that calls complete() without isReady()
returning true [6] and a test that calls write(), flush() and complete()
in sequence [7].

I am unable to explain why the behavior expected by the TCK is the exact
opposite of what was agreed multiple times by the EG. I will note that
the EG at the time, unlike now, had no access to the TCK source nor any
ability to update the spec document or Javadoc.

Given the behavior expected by the TCK, I don't think we have any choice
but to allow close(), complete() and dispatch() while async read/writes
are in progress.

I'll leave this thread for a couple of days in case anyone has any
further comments. Assuming no further discussion is required, I'll open
an issue and provide a PR to update the Javadoc and spec document
towards the end of this week / beginning of next.

Mark




[1]
https://github.com/apache/tomcat/blame/main/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java#L823

[2]
https://github.com/apache/tomcat/commit/690eb5c8d25d0fe26835386057bd455257cbdb86

[3]
https://download.oracle.com/javaee-archive/servlet-spec.java.net/jsr340-experts/2011/09/0056.html

[4]
https://download.oracle.com/javaee-archive/servlet-spec.java.net/jsr340-experts/2013/04/0569.html

[5]
https://download.oracle.com/javaee-archive/servlet-spec.java.net/jsr369-experts/2015/11/0312.html

[6]
https://github.com/jakartaee/platform-tck/blob/master/src/com/sun/ts/tests/servlet/api/jakarta_servlet_http/writelistener/TestListener.java

[7]
https://github.com/jakartaee/platform-tck/blob/master/src/com/sun/ts/tests/servlet/spec/async/TestServlet.java#L45
_______________________________________________
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