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!
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 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.
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