On Wed, 12 Nov 2014 11:27:13 +0100 (CET)
Daniel Stenberg <daniel_at_haxx.se> wrote:

> On Tue, 11 Nov 2014, Daniel Stenberg wrote:
>
> > My suggested alternative simplified code would be:
>
> I'm attaching the full (2/7) patch modified as I've suggested, using
> no additional struct members.

Sorry for the late reply.

I believe that your patch has two bugs:

1)

+ if(data->mstate != CURLM_STATE_DONE)
+ multistate(data, CURLM_STATE_DONE);
+ else
+ /* if already in DONE, go COMPLETED */
+ multistate(data, CURLM_STATE_COMPLETED);

as stated before in a reply from me, this is incorrect;
when data->mstate == CURLM_STATE_DONE here, then ONLY
multistate(data, CURLM_STATE_DONE) was called so far, which
only set data->mstate to CURLM_STATE_DONE and nothing else
was done yet: we certainly should still *run* that state!

Therefore instead of the above five lines, just do:

+ multistate(data, CURLM_STATE_DONE);

2)

- if((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
+ if(!conn->bits.close &&
+ (conn->handler->protocol & PROTO_FAMILY_HTTP) &&

That disables pipelining completely for the site in question no? It
seems that this is way too soon and too strong.

ConnectionExists() should return an existing connection
that can be reused. If the needle that is being passed is marked for
close - for example because it is a fresh connection that wasn't used
yet (or because on error happened on it), then you do NOT want to
ALWAYS create yet another fresh connection. For example, you don't want
that when it is a fresh connection, and you don't want that when some
other connection exists in the bundle that can be reused.

So, you most certainly need to fix the patch by testing bits.close at
the exact same place as I did. Then at least you'd still reuse existing
connections that are just fine.

I don't like this-- but at least it seems to be what you intend (which
is different from a bug).

-- 
Carlo Wood <carlo_at_alinoe.com>
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html