[jsr369-experts] isFinished() and onAllDataRead() clarifications (was: Re: Clarify onAllDataRead ?)

From: Edward Burns <edward.burns_at_oracle.com>
Date: Fri, 6 Feb 2015 14:08:59 -0800

I'll respond to the other portion of this thread on a separate message.

>>>>> On Fri, 2 Jan 2015 18:39:21 +0100, Greg Wilkins <gregw_at_intalio.com> said:

GW> I think we need to clarify when onAllDataRead should be called.
GW> My understanding is that once isReady() has returned false, a callback is
GW> scheduled:

GW> - If the IO that arrives is data, then onDataAvailable() is called.
GW> - If the IO that arrives is EOF, then onAllDataRead() is called.

GW> But it is possible for the code within onDataAvailable to read, call
GW> isReady() and get a true result and then read -1 for the EOF. In that
GW> case, should onAllDataRead() be called? If so should it wait until the
GW> call to onDataAvailable() has returned?

GW> I can see it two ways:

GW> Either we have a callback paradigm that is triggered by isReady()==false
GW> and that once that occurs there should be one and only one callback to
GW> either onDataAvailable or to onAllDataRead()


GW> onAllDataRead() is a separate callback paradigm to the onDataAvailable and
GW> will be called regardless once EOF is received.

GW> I think the former is simpler to implement, but might cause surprises for
GW> application developers who expect it always to be called.

GW> The problem with the latter is that it means that an onAllDataRead()
GW> callback can come even if isReady()!=false. This breaks the fundamental
GW> callback paradigm that a callback will only occur if isReady() has called
GW> false.

GW> thoughts?

>>>>> On Sun, 4 Jan 2015 17:27:01 -0500 (EST), Stuart Douglas <sdouglas_at_redhat.com> said:

SD> My gut feeling is that onAllDataRead should always be called,
SD> regardless of if a -1 was returned from onDataAvailable.

>>>>> On Mon, 12 Jan 2015 09:57:28 +0000, Mark Thomas <markt_at_apache.org> said:

MT> Agreed.

SD> In this case I think the obvious thing to do is to immediately call
SD> onAllDataRead after onDataAvailable has finished, regardless of if
SD> the user has called isReady(). I know this is a slightly different
SD> callback semantic, but I don't think it really matters.

MT> Also agreed.

SD> If you don't call onAllDataRead in all cases then you have a
SD> situation where a user has to handle EOF in two different places (I
SD> know they can just delegate to the onAllDataRead method, but it
SD> still seems a bit odd).

GW> As a secondary question, if EOF has arrived, but neither -1 has been read
GW> or onAllDataRead() called, should isFinished() return true?

SD> I don't think it should return true until this notification has been
SD> delivered to the user. Otherwise some layer of code could check this
SD> flag and take some action based on the isFinished flag, however the
SD> read listener may not have been notified yet. It also seems counter
SD> intuitive that callbacks could still be pending when isFinished()
SD> returns true.

>>>>> On Wed, 7 Jan 2015 17:39:49 -0800, Wenbo Zhu <wenboz_at_google.com> said:

WZ> I would expect all the above behavior too.

MT> Tomcat doesn't do this currently. We actually use the return value of
MT> isFinished() to check if onAllDataRead() should be called. However, I've
MT> no objection to clarifying the behaviour of isFinished() so it returns
MT> false until onAllDataRead() has completed.

The existing spec for isFinished() is:

Spec> isFinished()

Spec> Returns true when all data from the stream has been read else it
Spec> returns false.

Mark, are you suggesting we should amend this text to be something
similar to the following?

ProposedSpec> Returns true when all data from the stream has been else
ProposedSpec> it returns false. If called before onAllDataRead() has
ProposedSpec> returned to the container, it will also return false.

>>>>> On Thu, 8 Jan 2015 11:51:47 +0100, Greg Wilkins <gregw_at_intalio.com> said:

GW> I guess it would be good to clarify that onAllDataRead will not be
GW> called whilst there is a call inside onDataAvailable.

The existing spec for onAllDataRead() is:

Spec> onAllDataRead()

Spec> Invoked when all data for the current request has been read.

Greg, are you suggesting we should amend this text to be something
similar to the following?

ProposedSpec> Invoked when all data for the current request has been
ProposedSpec> read. This method must not be invoked while the user code
ProposedSpec> is in the midst of having its onDataAvailable() invoked.


| edward.burns_at_oracle.com | office: +1 407 458 0017
| 21 days til DevNexus 2015
| 31 days til JavaLand 2015
| 41 days til CONFESS 2015