On Mon, 20 Sep 2010, Dirk Manske wrote:

In url.c the dead connection is detected, than Curl_disconnect is called which calls Curl_expire. And this purges the timeout of the new request.

Without thinking about side effects I would remove the Curl_expire(0) call in Curl_disconnect. Or the timeouts must be re-added at some other place.

Ah yes. There are timeouts now set that will be removed by those explicit calls to Curl_disconnect.

Actually, this makes me realize that the removal of the timeouts should not be done in Curl_disconnect, they should probably instead simply get done in curl_easy_cleanup() and when the CURLM_STATE_COMPLETED multi state has been reached, as they are in fact set on a per easy-handle strictly speaking and not per connection. This is also true for the expire(0) call from within Curl_done() which suffers from the same problem.

I'll run some tests with a patch like attached.

You where right, the real bug is in hiperfifo. In function check_run_count the numbers of previous and still running requests are compared. curl_multi_info_read is only called "if (g->prev_running > g->still_running)" But if the request fails early then the running counter wasn't increased.

(Not only a fast "connection refused" can cause trouble, but also using a unsupported protocol etc.)

So applications have to service an own counter, or simply call curl_multi_info_read and see if there are messages.

Yes, we should fix the hiperfifo example accordingly (and probably a few other examples as well). In fact, this logic flaw was one of the major reasons for the curl_multi_info_read overhaul recently that now has made it really fast. It is really the only reliable way to detect that an easy handle is completed.

--

 / daniel.haxx.se
diff --git a/lib/multi.c b/lib/multi.c
index e857392..875e136 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1594,6 +1594,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       /* Important: reset the conn pointer so that we don't point to memory
          that could be freed anytime */
       easy->easy_conn = NULL;
+
+      Curl_expire(data, 0); /* stop all timers */
       break;
 
     case CURLM_STATE_MSGSENT:
diff --git a/lib/url.c b/lib/url.c
index 7fe713d..1b65a92 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -481,6 +481,8 @@ CURLcode Curl_close(struct SessionHandle *data)
   }
 #endif
 
+  Curl_expire(data, 0); /* shut off timers */
+
   if(m)
     /* This handle is still part of a multi handle, take care of this first
        and detach this handle from there. */
@@ -2579,7 +2581,6 @@ CURLcode Curl_disconnect(struct connectdata *conn)
                   NULL, Curl_scan_cache_used);
 #endif
 
-  Curl_expire(data, 0); /* shut off timers */
   Curl_hostcache_prune(data); /* kill old DNS cache entries */
 
   {
@@ -5131,8 +5132,6 @@ CURLcode Curl_done(struct connectdata **connp,
   conn = *connp;
   data = conn->data;
 
-  Curl_expire(data, 0); /* stop timer */
-
   if(conn->bits.done)
     /* Stop if Curl_done() has already been called */
     return CURLE_OK;
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to