cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH 2/7] pipelining: Fix connection handling under timeouts.

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Tue, 11 Nov 2014 23:40:09 +0100 (CET)

On Tue, 11 Nov 2014, Carlo Wood wrote:

> It is unclear to me if I answered your question or not, or if you still
> wonder the same thing.

My question is: Can't we simplify the check involving
data->multi_do_connection_id to instead just check in which state the easy
handle is?

> + if(data->multi_do_connection_id == data->easy_conn->connection_id) {
> + connclose(data->easy_conn, "Disconnected with pending data");
> + /* Make sure this connection is no longer considered for
> + pipelining. */
> + data->easy_conn->bits.timedout = TRUE;
> + }
>
> I don't understand what you mean with "seems to check that the timeout
> happens for the transfer that issued the most recent request on the
> connection". Why?

Argh. Sorry, I realize now when I reread the code that I somehow confused
matters and thought the 'data' there would be the multi handle and not the
easy handle. That was stupid. So no, those additional problems were just
figments of my imagination.

My suggested alternative simplified code would be:

   if((data->mstate >= CURLM_STATE_DO) &&
     (data->mstate <= CURLM_STATE_DONE)) {
     connclose(data->easy_conn, "Disconnected with pending data");
   }

... as I still question that "bits.timedout" is necessary because I think we
shouldn't add requests to a connection that is marked for close so connclose()
should be enough. I thus think IsPipeliningPossible() should return FALSE if
conn->bits.close is set to make sure it doesn't happen.

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2014-11-11