users@servlet-spec.java.net

[servlet-spec users] [jsr369-experts] Re: Servlet 4.0 Spec comments

From: Edward Burns <edward.burns_at_oracle.com>
Date: Tue, 8 Dec 2015 11:40:33 -0800

Copying from mail sent to users_at_servlet-spec.java.net.

>>>>> On Tue, 1 Dec 2015 18:00:29 -0500, "Martin Mulholland" <mmulholl_at_us.ibm.com> said:

MM> Section 3.7:
MM> onDataAvailable(). The onDataAvailable method is invoked
MM> on the ReadListener when data is available to read from the incoming
MM> request stream. The container will invoke the method the first time when
MM> data is available to read. The container will subsequently invoke the
MM> onDataAvailable method if and only if isReady method on ServletInputStream
MM> , described below, returns true.

MM> The last word in this has been changed from false to true which
MM> changes it from a container perspective to an app perspective. This
MM> should be clarified. I suggest:
MM> The container will subsequently invoke the onDataAvaialble method if and
MM> only if isReady method on ServletOutputStream, described below, previously
MM> returned false but will now return true.

First don't you mean ServletInputStream? Second, there is no precedent
for including what amounts to a textual changebar in the spec text.

>>>>> On Wed, 2 Dec 2015 13:24:51 +1100, Greg Wilkins <gregw_at_webtide.com> said:

GW> Good catch!

GW> This is just wrong! It is not a perspective thing. A container will
GW> NEVER call onDataAvailable after the first invocation unless isReady has
GW> been called AND it returned false.

This was done under SERVLET_SPEC-127. I have asked Shing-wai to explain
the rationale for that change. In the meantime, I don't understand your
assertion Greg. If the container calls ServletInputStream.isReady() and
it returns false, then it doesn't make sense for the container to call
ReadListener.onDataAvailable() because *data is not available*. Right?
Please help me understand.

MM> Section 3.8:

Spec> Unless explicitly excluded, containers must support server push as
Spec> specified in HTTP/2 specification section Server Push. Containers
Spec> must support server push if the client is capable of speaking
Spec> HTTP/2, unless the client has explicitly disabled server push by
Spec> sending a SETTINGS_ENABLE_PUSH setting value of 0 (zero).

MM> First sentence appears to suggest that a container can not support HTTP/2
MM> and still be compliant with Servlet 4.0, but the second sentence implies
MM> the opposite. Need to clarify this.

I can clarify it. The SETTINGS_ENABLE_PUSH value is TCP connection
specific, so it has a very limited scope. Here is my proposed text:

Proposed38> Servlet 4.0 containers must support server push as specified
Proposed38> in HTTP/2 specification section Server Push. Containers must
Proposed38> enable server push if the client is capable of speaking
Proposed38> HTTP/2, unless the client has explicitly disabled server
Proposed38> push by sending a SETTINGS_ENABLE_PUSH setting value of 0
Proposed38> (zero) for the current connection. In that case, for that
Proposed38> connection only, server push must not be enabled.

MM> Need to add information about When push is called the container
MM> sends the request to the client and subsequently sends the content
MM> of the request, on a separate stream, if the client accepts.

I chose to not be explicit about that and relied instead on the
transitive inclusion of HTTP/2 spec section "Server Push".

MM> If the push fails how should the failure reported?

The result of the push is not reflected in the Servlet API. Do you
think it should be?

MM> Use of streams. Each accepted push is sent on a different stream and a
MM> different stream from the response for the inbound request.
MM> To support async servlets should we consider a mechanism for the
MM> dispatched thread to access an existing push builder.

The javadoc for PushBuilder.push() states:

Spec> Push a resource given the current state of the builder, returning
Spec> immediately without blocking.

This implies the push is a "fire and forget" type interaction.
Therefore, I don't think we need to expose a way to get a handle on an
existing PushBuilder.

MM> Section 5.3 - onWritePossible

Spec> void onWritePossible(). When a WriteListener is registered with
Spec> the ServletOutputStream, this method will be invoked by the
Spec> container the first time when it is possible to write data. The
Spec> container will subsequently invoke the onWritePossible method if
Spec> and only if isReady method on ServletOutputStream, described
Spec> below, returns true.

Again, this is from SERVLET_SPEC-127.

MM> The last word in this has been changed from false to true which
MM> changes it from a container perspective to and app perspective. This
MM> should be clarified. I suggest:
MM> The container will subsequently invoke the onWritePossible method if and
MM> only if isReady method on ServletOutputStream, described below, previously
MM> returned false but will now return true.

Again, I there is no precedent for textual changebars.

MM> Section 5.3 - setWriteListener()

Spec> It is illegal to switch to the traditional blocking IO at that
Spec> point.

MM> Add a clarification to say how such a illegal switch is reported.
MM> IOException from OutputStream write methods?

I propose adding the following text after "at that point."

Proposed53> The use of IO related method calls after this illegal switch
Proposed53> to traditional blocking IO produces unspecified results.


-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017