We introduce a loop in lib/multi.c around all calls to multi_runsingle() and

simply check for CURLM_CALL_MULTI_PERFORM internally. This has the added
benefit that this goes in line with my long-term wishes to get rid of the
CURLM_CALL_MULTI_PERFORM all together from the public API.

5 files changed, 92 insertions, 40 deletions

diff --git a/CHANGES b/CHANGES
index cd4634e8f..b5a512a78 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,63 @@
 
                                   Changelog
 
+Daniel Stenberg (1 Feb 2010)
+- Using the multi_socket API, it turns out at times it seemed to "forget"
+  connections (which caused a hang). It turned out to be an existing (7.19.7)
+  bug in libcurl (that's been around for a long time) and it happened like
+  this:
+
+  The app calls curl_multi_add_handle() to add a new easy handle, libcurl will
+  then set it to timeout in 1 millisecond so libcurl will tell the app about
+  it.
+
+  The app's timeout fires off that there's a timeout, the app calls libcurl as
+  we so often document it:
+
+  do {
+   res = curl_multi_socket_action(... TIMEOUT ...);
+  } while(CURLM_CALL_MULTI_PERFORM == res);
+
+  And this is the problem number one:
+
+  When curl_multi_socket_action() is called with no specific handle, but only
+  a timeout-action, it will *only* perform actions within libcurl that are
+  marked to run at this time. In this case, the request would go from INIT to
+  CONNECT and return CURLM_CALL_MULTI_PERFORM. When the app then calls libcurl
+  again, there's no timer set for this handle so it remains in the CONNECT
+  state. The CONNECT state is a transitional state in libcurl so it reports no
+  sockets there, and thus libcurl never tells the app anything more about that
+  easy handle/connection.
+
+  libcurl _does_ set a 1ms timeout for the handle at the end of
+  multi_runsingle() if it returns CURLM_CALL_MULTI_PERFORM, but since the loop
+  is instant the new job is not ready to run at that point (and there's no
+  code that makes libcurl call the app to update the timout for this new
+  timeout). It will simply rely on that some other timeout will trigger later
+  on or that something else will update the timeout callback. This makes the
+  bug fairly hard to repeat.
+
+  The fix made to adress this issue:
+
+  We introduce a loop in lib/multi.c around all calls to multi_runsingle() and
+  simply check for CURLM_CALL_MULTI_PERFORM internally. This has the added
+  benefit that this goes in line with my long-term wishes to get rid of the
+  CURLM_CALL_MULTI_PERFORM all together from the public API.
+
+  The downside of this fix, is that the counter we return in 'running_handles'
+  in several of our public functions then gets a slightly new and possibly
+  confusing behavior during times:
+
+  If an app adds a handle that fails to connect (very quickly) it may just
+  as well never appear as a 'running_handle' with this fix. Previously it
+  would first bump the counter only to get it decreased again at next call.
+  Even I have used that change in handle counter to signal "end of a
+  transfer". The only *good* way to find the end of a individual transfer
+  is calling curl_multi_info_read() to see if it returns one.
+
+  Of course, if the app previously did the looping before it checked the
+  counter, it really shouldn't be any new effect.
+
 Yang Tse (26 Jan 2010)
 - Constantine Sapuntzakis' and Joshua Kwan's work done in the last four months
   relative to the asynchronous DNS lookups, along with with some integration
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 3140a1903..d8a1fe4d9 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -50,6 +50,7 @@ This release includes the following bugfixes:
  o curl_multi_fdset() would return -1 too often during SCP/SFTP transfers
  o FTP file size checks with ASCII transfers
  o HTTP Cookie: headers sort cookies based on specified path lengths
+ o CURLM_CALL_MULTI_PERFORM fix for multi socket timeout calls
 
 This release includes the following known bugs:
 
diff --git a/docs/libcurl/curl_multi_perform.3 b/docs/libcurl/curl_multi_perform.3
index 746ff3de0..daf15c478 100644
--- a/docs/libcurl/curl_multi_perform.3
+++ b/docs/libcurl/curl_multi_perform.3
@@ -22,25 +22,26 @@ changed from the previous call (or is less than the amount of easy handles
 you've added to the multi handle), you know that there is one or more
 transfers less "running". You can then call \fIcurl_multi_info_read(3)\fP to
 get information about each individual completed transfer, and that returned
-info includes CURLcode and more.
+info includes CURLcode and more. If an added handle fails very quickly, it may
+never be counted as a running_handle.
 
 When \fIrunning_handles\fP is set to zero (0) on the return of this function,
 there is no longer any transfers in progress.
 .SH "RETURN VALUE"
 CURLMcode type, general libcurl multi interface error code.
 
-If you receive \fICURLM_CALL_MULTI_PERFORM\fP, this basically means that you
-should call \fIcurl_multi_perform\fP again, before you select() on more
-actions. You don't have to do it immediately, but the return code means that
-libcurl may have more data available to return or that there may be more data
-to send off before it is "satisfied". Do note that \fIcurl_multi_perform(3)\fP
-will return \fICURLM_CALL_MULTI_PERFORM\fP only when it wants to be called
-again \fBimmediately\fP. When things are fine and there is nothing immediate
-it wants done, it'll return \fICURLM_OK\fP and you need to wait for \&"action"
-and then call this function again.
+Before version 7.20.0: If you receive \fICURLM_CALL_MULTI_PERFORM\fP, this
+basically means that you should call \fIcurl_multi_perform\fP again, before
+you select() on more actions. You don't have to do it immediately, but the
+return code means that libcurl may have more data available to return or that
+there may be more data to send off before it is "satisfied". Do note that
+\fIcurl_multi_perform(3)\fP will return \fICURLM_CALL_MULTI_PERFORM\fP only
+when it wants to be called again \fBimmediately\fP. When things are fine and
+there is nothing immediate it wants done, it'll return \fICURLM_OK\fP and you
+need to wait for \&"action" and then call this function again.
 
-NOTE that this only returns errors etc regarding the whole multi stack. Problems
-still might have occurred on individual transfers even when this
+This function only returns errors etc regarding the whole multi stack.
+Problems still might have occurred on individual transfers even when this
 function returns \fICURLM_OK\fP.
 .SH "TYPICAL USAGE"
 Most applications will use \fIcurl_multi_fdset(3)\fP to get the multi_handle's
diff --git a/docs/libcurl/curl_multi_socket_action.3 b/docs/libcurl/curl_multi_socket_action.3
index cc53dbbb0..65eef4e3f 100644
--- a/docs/libcurl/curl_multi_socket_action.3
+++ b/docs/libcurl/curl_multi_socket_action.3
@@ -22,8 +22,8 @@ CURL_CSELECT_ERR. When the events on a socket are unknown, pass 0 instead, and
 libcurl will test the descriptor internally.
 
 At return, the integer \fBrunning_handles\fP points to will contain the number
-of still running easy handles within the multi handle. When this number
-reaches zero, all transfers are complete/done. Note that when you call
+of running easy handles within the multi handle. When this number reaches
+zero, all transfers are complete/done. When you call
 \fIcurl_multi_socket_action(3)\fP on a specific socket and the counter
 decreases by one, it DOES NOT necessarily mean that this exact socket/transfer
 is the one that completed. Use \fIcurl_multi_info_read(3)\fP to figure out
@@ -89,19 +89,16 @@ The \fIuserp\fP argument is a private pointer you have previously set with
 .SH "RETURN VALUE"
 CURLMcode type, general libcurl multi interface error code.
 
-Legacy: If you receive \fICURLM_CALL_MULTI_PERFORM\fP, this basically means
-that you should call \fIcurl_multi_socket_action(3)\fP again, before you wait
-for more actions on libcurl's sockets. You don't have to do it immediately,
-but the return code means that libcurl may have more data available to return
-or that there may be more data to send off before it is "satisfied".
+Before version 7.20.0: If you receive \fICURLM_CALL_MULTI_PERFORM\fP, this
+basically means that you should call \fIcurl_multi_socket_action(3)\fP again
+before you wait for more actions on libcurl's sockets. You don't have to do it
+immediately, but the return code means that libcurl may have more data
+available to return or that there may be more data to send off before it is
+"satisfied".
 
-In modern libcurls (from around 7.19.0 or later),
-\fICURLM_CALL_MULTI_PERFORM\fP or \fICURLM_CALL_MULTI_SOKCET\fP will not be
-returned and no application needs to care about them.
-
-NOTE that the return code from this function is for the whole multi stack.
-Problems still might have occurred on individual transfers even when one of
-these functions return OK.
+The return code from this function is for the whole multi stack.  Problems
+still might have occurred on individual transfers even when one of these
+functions return OK.
 .SH "TYPICAL USAGE"
 1. Create a multi handle
 
diff --git a/lib/multi.c b/lib/multi.c
index d9414b9eb..5d19b089b 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -1568,17 +1568,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
     multi->num_msgs++; /* increase message counter */
   }
 
-  if(CURLM_CALL_MULTI_PERFORM == result)
-    /* Set the timeout for this handle to expire really soon so that it will
-       be taken care of even when this handle is added in the midst of
-       operation when only the curl_multi_socket() API is used. During that
-       flow, only sockets that time-out or have actions will be dealt
-       with. Since this handle has no action yet, we make sure it times out to
-       get things to happen. Also, this makes it less important for callers of
-       the curl_multi_* functions to bother about the CURLM_CALL_MULTI_PERFORM
-       return code, as long as they deal with the timeouts properly. */
-    Curl_expire(easy->easy_handle, 1);
-
   return result;
 }
 
@@ -1597,7 +1586,10 @@ CURLMcode curl_multi_perform(CURLM *multi_handle, int *running_handles)
   while(easy != &multi->easy) {
     CURLMcode result;
 
-    result = multi_runsingle(multi, easy);
+    do
+      result = multi_runsingle(multi, easy);
+    while (CURLM_CALL_MULTI_PERFORM == result);
+
     if(result)
       returncode = result;
 
@@ -1956,7 +1948,9 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
       if(data->set.one_easy->easy_conn)  /* set socket event bitmask */
         data->set.one_easy->easy_conn->cselect_bits = ev_bitmask;
 
-      result = multi_runsingle(multi, data->set.one_easy);
+      do
+        result = multi_runsingle(multi, data->set.one_easy);
+      while (CURLM_CALL_MULTI_PERFORM == result);
 
       if(data->set.one_easy->easy_conn)
         data->set.one_easy->easy_conn->cselect_bits = 0;
@@ -1985,7 +1979,9 @@ static CURLMcode multi_socket(struct Curl_multi *multi,
 
     /* the first loop lap 'data' can be NULL */
     if(data) {
-      result = multi_runsingle(multi, data->set.one_easy);
+      do
+        result = multi_runsingle(multi, data->set.one_easy);
+      while (CURLM_CALL_MULTI_PERFORM == result);
 
       if(CURLM_OK >= result)
         /* get the socket(s) and check if the state has been changed since