curl-library
Re: [PATCH] Better pipelining, round two
Date: Wed, 16 Jan 2008 19:09:29 +0300
2008/1/16, Daniel Stenberg <daniel_at_haxx.se>:
> On Wed, 16 Jan 2008, Dmitry Kurochkin wrote:
>
> > Pipelining patch v3 attached.
>
> Thanks! I committed it just now with some minor edits:
Great!
> > Now I plan to try some real world usage with pipelining. I want to use
> > it in darcs. But this will take some time.
>
> Great! Also, if you feel brave and want to dig further into the lovely wonders
> of libcurl and pipelining, I have this outstanding bug report from June
> nagging on my conscience:
>
> http://curl.haxx.se/mail/lib-2007-06/0336.html
I am able to reproduce this with version curl-7.16.3 and curl-7.17.1.
But curl from CVS (without my patch) does not segfault so I believe
that problem is fixed.
Comparing debug output of this test of CVS version with and without my
patch I found a bug.
Fix is attached.
Also I have questions regarding curl_multi_remove_handle() function.
At line 590 we check for easy->state > CURLM_STATE_DO. Should not this
be >= so that we check for handle *started* sending request and not
completed sending request?
Also, should not there be a check for non pipelining case like:
if (started sending req and not response)
set close flag
? This will prevent us from leaving a persistent connection in the
middle of transaction, so we will not reuse it.
And regarding the pipelining check at line 589. Am I correct that it
basically returns success to the user but quietly tries to complete
request/response so that other requests in pipeline can proceed? If
this is true isn't a better option to close the connection, and cancel
all pipeline requests? This may be not good for performance, since
another connection is done but it is better than getting problems when
user cleanups that handle... Or I am just missing a point here?
Regards,
Dmitry
- text/x-diff attachment: pipelining_call_multi_perform.patch