----- 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.
>
> 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.
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.
>
> 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.
Stuart
>
> cheers
>
>
>
>
>
>
>
>
>
>
>
> On 27 November 2015 at 10:37, 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 10:16:21 AM
> > > Subject: [jsr369-experts] Calling complete with write pending
> > >
> > > For the complete method we still say:
> > >
> > > "It is legal to call this [complete] method anytime after a call to
> > > ServletRequest.startAsync() or
> > > ServletRequest.startAsync(ServletRequest, ServletResponse)
> > > and before a call to one of the dispatch methods."
> > >
> > > But that text was written before we added the async IO API which has
> > > introduced the ability to have statefulness like write/read callbacks
> > > pending.
> > >
> > >
> > > So for example if an app has done a write(...), then called isReady(),
> > > which has returned false, is it still OK to call complete()?
> >
> > I think it should be legal, although we should discourage it.
> >
> > If complete() is called then the request should be finished, if we throw
> > an exception instead then the most likely result is a request timeout (as
> > they will likely not call it again).
> >
> > I think we could also ask a similar question about
> > ServletOutputStream.close(), what happens if they call the close() method
> > when a write is pending? In Undertow we handle this by doing an async flush
> > on any data that remains to be written out (and as our complete()
> > implementation internally closes the stream this handling is the same for
> > complete()).
> >
> > I think that either way is kind of messy. We should definitively
> > discourage users from doing this, but if they do then IMHO we need to make
> > sure that we actually finish the request, instead of potentially letting it
> > time out.
> >
> > Stuart
> >
> > >
> > > Jetty's interpretation is no. a call to complete has an implicit write
> > > operation (flush buffers and/or send the last chunk). That write cannot
> > be
> > > done until the pending write is complete. Furthermore, if we do call
> > > complete when isReady() is false, we are in a race with a potential
> > onError
> > > callback.
> > >
> > > Thus I think that complete method can only be called in async IO mode if
> > > the write site isReady() method returns true.
> > >
> > > thoughts?
> > >
> > > --
> > > Greg Wilkins <gregw@webtide.com> CTO http://webtide.com
> > >
> >
>
>
>
> --
> Greg Wilkins <gregw@webtide.com> CTO http://webtide.com
>