hc-httpclient-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: Change in BasicFuture between 4.2 and 4.3
Date Fri, 30 Aug 2013 15:36:21 GMT
On Fri, 2013-08-30 at 15:20 +0200, Arnaud Quillaud wrote:
> Hello,
> 
> I have just switched http components from 4.2 to 4.3 and I'm 
> experiencing a change in behavior.
> 
> I'm using something like:
> 
>          Future<HttpResponse> f = httpClient.execute(httpRequest, callback);
>          if (async) {
>              return;
>          }
> 
>          HttpResponse resp = f.get();
>          ...
> 
> where callback implements FutureCallback<HttpResponse>
> 
> With 4.2, the completed()/failed()/cancelled() method would be called 
> first and f.get() would not return until it was done.
> With 4.3, the f.get() method returns before or in the middle of the 
> completed() method, which totally defies the purpose of using f.get().
> 
> I have tracked down this behavior change to a commit in 
> BasicFuture.java. In 4.2, the whole completed() method was synchronized:
> 
>          public synchronized boolean completed(final T result) {
>              if (this.completed) {
>                  return false;
>              }
>              this.completed = true;
>              this.result = result;
>              if (this.callback != null) {
>                  this.callback.completed(result);
>              }
>              notifyAll();
>              return true;
>          }
> 
> As a consequence, the notifyAll() method is called only after 
> callback.completed() is called
> 
> In 4.3 though, only a portion of the method is synchronized:
> 
>          public boolean completed(final T result) {
>          synchronized(this) {
>              if (this.completed) {
>                  return false;
>              }
>              this.completed = true;
>              this.result = result;
>              notifyAll();
>          }
>          if (this.callback != null) {
>              this.callback.completed(result);
>          }
>          return true;
>      }
> It then becomes possible for the get() method (which is also 
> synchronized on this) to return before this.callback.completed to be called.
> 
> Or did I misunderstood the use of the get() method ?
> 
> Thanks,
> 
> Arnaud Quillaud
> 

Arnaud

This change was made to this issue

https://issues.apache.org/jira/browse/HTTPCORE-331

Some people abuse FutureCallback#failed event to re-execute requests in
case of a failure. One can argue whether this is a sound design decision
or not, but it often could lead to deadlocks in the connection pool. 

At the same time I have to say that your assumption about a particular
order of execution was wrong. The Future<HttpResponse> instance
represents the future result of request execution, not that of callback
event. You ought not make any assumptions about their order of
execution. If your callback events are used to do some extra processing
of the response message you should implement this logic inside a custom
response consumer.   

Hope this helps

Oleg


---------------------------------------------------------------------
To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
For additional commands, e-mail: httpclient-users-help@hc.apache.org


Mime
View raw message