2008/1/17, Daniel Stenberg <daniel_at_haxx.se>:
> On Wed, 16 Jan 2008, Dmitry Kurochkin wrote:
>
> > Comparing debug output of this test of CVS version with and without my patch
> > I found a bug. Fix is attached.
>
> Applied! But you should probably cvs update since you used the older function
> name before my rename...

I know. I sent the patch from work. And I am behind http proxy which
restricts connections to 80 and 443 ports only. So I am not able to
connect to CVS.

> > 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?
>
> That sounds reasonable, yes. I think possibly it says > DO because the state
> is set to DO even before the request has been sent off, but is then kept at DO
> until entirely completed.
>
> > 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.
>
> That shouldn't be necessary since we do Curl_done() on the connection a bit
> further down.
>
> > 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?
>
> Yes, that's what it's all about. It marks the handle as wanted-to-be-removed
> but doesn't actually do it now but leaves it to be removed later when it can
> be removed without causing as much problems.

So curl_multi_remove_handle() returns CURLM_OK but in fact continues
to use easy handle. But documentation says:

Removing a handle while being used, will effectively halt all
transfers in progress.

Moreover of user does curl_easy_cleanup() on the handle we get a major
problem in Curl_close()... I think we should either return error code
from curl_multi_remove_handle() or really close connection and remove
handle.

Regards,
  Dmitry

>
> --
> Commercial curl and libcurl Technical Support: http://haxx.se/curl.html
>
Received on 2008-01-16