[AsyncHttpClient] order of Future completion and callback invocation

[Sangjin Lee <sj...@gmail.com>]

Currently the callback methods from AsyncHttpClientCallback get called
*after* the response future object is completed.  I think this causes a
subtle bug that may prevent the callback methods from being completed if one
uses both the callback and the future.
For example, consider the following case:

ResponseFuture future = client.invoke(..., callback); // callback is not
null
HttpResponseMessage response = future.get(); // this blocks until future is
complete

Since the future is completed before the callback is invoked, the caller
thread in this case gets unblocked before the callback is called.  If the
caller thread goes away, then there is possibility that the callback may not
be completed or may not even be called, depending on the timing.

This strikes me as a bug...  I propose we invoke the callback first and then
complete the future so the callbacks are guaranteed to be completed even if
future is used.  What do you think?  I'll open a JIRA issue if you guys
agree this is a bug.

Thanks,
Sangjin

Re: [AsyncHttpClient] order of Future completion and callback invocation

[Rick McGuire <ri...@gmail.com>]

Sangjin Lee wrote:
> Currently the callback methods from AsyncHttpClientCallback get called 
> *after* the response future object is completed.  I think this causes 
> a subtle bug that may prevent the callback methods from being 
> completed if one uses both the callback and the future.
>
> For example, consider the following case:
>
> ResponseFuture future = client.invoke(..., callback); // callback is 
> not null
> HttpResponseMessage response = future.get(); // this blocks until 
> future is complete
>
> Since the future is completed before the callback is invoked, the 
> caller thread in this case gets unblocked before the callback is 
> called.  If the caller thread goes away, then there is possibility 
> that the callback may not be completed or may not even be called, 
> depending on the timing.
>
> This strikes me as a bug...  I propose we invoke the callback first 
> and then complete the future so the callbacks are guaranteed to be 
> completed even if future is used.  What do you think?  I'll open a 
> JIRA issue if you guys agree this is a bug.
Yes, I agree.  If the callback is used, then we should ensure that it 
gets called appropriately. 

>
> Thanks,
> Sangjin
>

Re: [AsyncHttpClient] order of Future completion and callback invocation

[Sangjin Lee <sj...@gmail.com>]

I have opened a JIRA issue (GERONIMO-3751), and attached a suggested patch.
Please let me know what you think...

Thanks,
Sangjin

On Jan 15, 2008 11:49 AM, Sangjin Lee <sj...@gmail.com> wrote:

> Currently the callback methods from AsyncHttpClientCallback get called
> *after* the response future object is completed.  I think this causes a
> subtle bug that may prevent the callback methods from being completed if one
> uses both the callback and the future.
> For example, consider the following case:
>
> ResponseFuture future = client.invoke(..., callback); // callback is not
> null
> HttpResponseMessage response = future.get(); // this blocks until future
> is complete
>
> Since the future is completed before the callback is invoked, the caller
> thread in this case gets unblocked before the callback is called.  If the
> caller thread goes away, then there is possibility that the callback may not
> be completed or may not even be called, depending on the timing.
>
> This strikes me as a bug...  I propose we invoke the callback first and
> then complete the future so the callbacks are guaranteed to be completed
> even if future is used.  What do you think?  I'll open a JIRA issue if you
> guys agree this is a bug.
>
> Thanks,
> Sangjin
>
>