jsr369-experts@servlet-spec.java.net

[jsr369-experts] Re: [servlet-spec users] Re: Re: Re: Calling complete with write pending

From: Greg Wilkins <gregw_at_webtide.com>
Date: Fri, 27 Nov 2015 15:55:58 +1100

On 27 November 2015 at 12:11, Stuart Douglas <sdouglas_at_redhat.com> wrote:

>
>
> ----- Original Message -----
> > From: "Greg Wilkins" <gregw_at_webtide.com>
> > To: jsr369-experts_at_servlet-spec.java.net
> > Sent: Friday, 27 November, 2015 11:20:00 AM
> > Subject: [servlet-spec users] [jsr369-experts] Re: Re: Calling complete
> with write pending
> >
> > Stuart,
> >
> > indeed this is the same problem for close() as that too caries an implied
> > write (of cached content and/or last chunk). Probably sendError(),
> > flushBuffers() OutputStream.flush() as well.
> >
> > The problem I have with doing flushes/closes etc. within the complete()
> and
> > close() methods is that this makes them essentially blocking (as they
> have
> > to wait for IO to complete), which is precisely what we want to avoid by
> > going async. Apps that use async to prevent against thread starvation
> > DOS attacks may then become vulnerable if close() and complete() can
> block
> > on them!
>
> We actually do an async flush (so you many not be notified on error, but
> it prevents these kind of blocking issues). IMHO should definitely specify
> the behavior of these methods though, at the moment it is not really very
> well defined how they should work when a stream is in async mode.
>
>
So you could do

   write(veryLargeBuffer);
   complete();

and within the complete you do an async flush. So what does that mean
for any onComplete handlers? Are they called during the the write or after
it completes?



> >
> > Once we go async IO, we require the app developer to delay further calls
> to
> > write until isReady() is true, so surely more important methods like
> close
> > and complete with implied writes should also be required to wait?
>
> I am more concerned about generic error handling code that attempts to
> finish the
> request on error, that may not have knowledge of the current write state.
>

Well generic error handling is already pretty well broken by async IO.
I'm betting that many containers will fail on sendError with an error page
mapping to a JSP that has no ability to write the error page in async mode.

I'm not saying it is not a problem we should solve... just saying that it
is already a bit messed up. I'm betting that only very specific error
handling will work reliably for servlets that do async IO




> Also from a technical point of view there is no real reason why close()
> and complete() cannot be queued till after the data is written. We require
> write() to wait because otherwise we would need to buffer a potentially
> unbounded amount of data, however we don't have the same issue with these
> methods.
>

The problem with queing the complete or close is that it is hard to know
what to do with errors/timeout that happen to the write when you already
have called complete.

What if the client is reading the veryLargeBuffer above is really slowly so
that the async timeout expires? Do you callback onTimeout then? this
would be strange as complete has already been called?

> I know that can be difficult, but async is hard and it does us no good to
> > pretend otherwise. Allowing blocking calls to be interspersed with async
> > calls is fraught. We had that discussion back when we were considering
> if
> > we could revert to blocking IO - and that proved to be too difficult.
>
> I just think that there is a potential for problems if error handling code
> attempts to end the request and it fails. A simple example is what happens
> if a request times out while a write is in progress? According to the spec
> the container needs to call complete(), however if we have made this
> illegal what happens then?
>
> I just think that if a user has indicated that they want to complete the
> request we need to make sure it completes, and does not just leave a
> hanging connection.
>

Well it should never be left hanging, as it should eventually timeout.

But I agree it would be nice to have a simple call that would bring the
request cycle to an end no matter what. Perhaps we need to say that if
complete is called with a pending write then that write is aborted -
onError is called with a WriteInterruptedException and then the complete
takes place (if the onError has not already called complete or dispatch).

ie I think it would be safer for complete to really be complete. Queuing
complete just leaves us open to more strangeness for events that happen
while it is in the queue.

So if you want your write to work, wait for isReady()==true before calling
complete(). But if you call complete() before then, it will complete, but
you may truncate the response.


cheers







-- 
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com