On Dec 8, 2015, at 2:40 PM, Edward Burns <edward.burns_at_oracle.com> wrote:
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.
ED> First don't you mean ServletInputStream? Second, there is no
precedent
ED>for including what amounts to a textual changebar in the spec text.
Sorry, yes I meant ServletInputStream.
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.
ED > This was done under SERVLET_SPEC-127. I have asked Shing-wai to
explain
ED> the rationale for that change. In the meantime, I don't understand
your
ED> assertion Greg. If the container calls ServletInputStream.isReady()
and
ED> it returns false, then it doesn't make sense for the container to call
ED >ReadListener.onDataAvailable() because *data is not available*.
Right?
ED >Please help me understand.
The way I interpreted the spec before was that only after an app calls
isReady() and
it returns false should the container be responsible for calling
onDataAvailable again
- when more data is available. If the app returns without having called
isReady() and
got false the container is not on the hook to call onDataAvailable again.
I have to
come up with a completely different interpretation with false changed to
true. I
agree it makes no sense to call onDataAvailable unless isReady() will
return true, but
this section of the spec previously made a much more important point than
that.
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.
ED> I can clarify it. The SETTINGS_ENABLE_PUSH value is TCP connection
ED> 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.
Thanks, it is quite clear now. Containers have to support HTTP2 to comply
with Servlet 4.0.
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.
ED> I chose to not be explicit about that and relied instead on the
ED> transitive inclusion of HTTP/2 spec section "Server Push”.
OK, I just think it helps a reader if the basics are briefly explained.
MM> If the push fails how should the failure reported?
ED> The result of the push is not reflected in the Servlet API. Do you
ED> think it should be?
I don’t have a suggestion on how to do this but it seems an issue if a
servlet
could start doing multiple push requests, all of which fail for the same
reason.
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.
Spec also says for the Push method:
Spec > Before returning from the method the builder has its path, etag,
Spec > and lastModified fields nulled. All other fields are left as-is for
possible
Spec> reuse in another push.
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.
That is an improvement over silence, but leaves us in a position where
containers can behave differently making apps less portable.
Thank you,
Martin Mulholland.
WebSphere Application Server Web Tier Architect
email: mmulholl_at_us.ibm.com
IBM RTP, PO BOX 12195, 503/C227,
3039 Cornwallis Rd, RTP, NC 27709-2195
t/l 444-4319, external (919)-254-4319