- Jeff Pohlmeyer identified two problems: first a rather obscure problem with

  the multi interface and connection re-use that could make a
  curl_multi_remove_handle() ruin a pointer in another handle.

  The second problem was less of an actual problem but more of minor quirk:
  the re-using of connections wasn't properly checking if the connection was
  marked for closure.

6 files changed, 30 insertions, 20 deletions

diff --git a/CHANGES b/CHANGES
index c7097881c..aa11640b5 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,15 @@
 
                                   Changelog
 
+Daniel (18 February 2007)
+- Jeff Pohlmeyer identified two problems: first a rather obscure problem with
+  the multi interface and connection re-use that could make a
+  curl_multi_remove_handle() ruin a pointer in another handle.
+
+  The second problem was less of an actual problem but more of minor quirk:
+  the re-using of connections wasn't properly checking if the connection was
+  marked for closure.
+
 Daniel (16 February 2007)
 - Duncan Mac-Vicar Prett and Michal Marek reported problems with resetting
   CURLOPT_RANGE back to no range on an easy handle when using FTP.
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index b19c551e7..59796b20a 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -23,6 +23,7 @@ This release includes the following bugfixes:
  o socks5 works
  o builds fine with VC2005
  o CURLOPT_RANGE set to NULL resets the range for FTP
+ o curl_multi_remove_handle() rare crash
 
 This release includes the following known bugs:
 
@@ -40,6 +41,7 @@ This release would not have looked like this without help, code, reports and
 advice from friends like these:
 
  Yang Tse, Manfred Schwarb, Michael Wallner, Jeff Pohlmeyer, Shmulik Regev,
- Rob Crittenden, Robert A. Monat, Duncan Mac-Vicar Prett, Michal Marek
+ Rob Crittenden, Robert A. Monat,  Dan Fandrich, Duncan Mac-Vicar Prett,
+ Michal Marek
 
         Thanks! (and sorry if I forgot to mention someone)
diff --git a/docs/KNOWN_BUGS b/docs/KNOWN_BUGS
index d2cfbcf3e..3155c45f5 100644
--- a/docs/KNOWN_BUGS
+++ b/docs/KNOWN_BUGS
@@ -7,9 +7,6 @@ may have been fixed since this was written!
   --ftp-ssl-ccc on some servers.  Recipe and instructions here:
   http://curl.haxx.se/mail/lib-2007-01/0210.html
 
-41. Jeff Pohlmeyer's curl_multi_socket crashing case. Recipe and instructions
-  here: http://curl.haxx.se/mail/lib-2007-01/0022.html
-
 40. HTTP Pipelining, NULL content
   http://curl.haxx.se/bug/view.cgi?id=1631566
 
diff --git a/lib/http.c b/lib/http.c
index 8b9e5665c..638c5fad2 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1403,6 +1403,10 @@ CURLcode Curl_http_connect(struct connectdata *conn, bool *done)
 
   data=conn->data;
 
+  /* We default to persistent connections. We set this already in this connect
+     function to make the re-use checks properly be able to check this bit. */
+  conn->bits.close = FALSE;
+
   /* If we are not using a proxy and we want a secure connection, perform SSL
    * initialization & connection now.  If using a proxy with https, then we
    * must tell the proxy to CONNECT to the host we want to talk to.  Only
@@ -1674,9 +1678,6 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
   else
     http = data->reqdata.proto.http;
 
-  /* We default to persistent connections */
-  conn->bits.close = FALSE;
-
   if ( (conn->protocol&(PROT_HTTP|PROT_FTP)) &&
        data->set.upload) {
     httpreq = HTTPREQ_PUT;
diff --git a/lib/multi.c b/lib/multi.c
index d32172288..49db7d064 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -542,11 +542,10 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle,
       easy->easy_handle->dns.hostcachetype = HCACHE_NONE;
     }
 
-    /* if we have a connection we must call Curl_done() here so that we
-       don't leave a half-baked one around */
-    if(easy->easy_conn) {
-      /* Set up the association right */
-      easy->easy_conn->data = easy->easy_handle;
+    /* we must call Curl_done() here (if we still "own it") so that we don't
+       leave a half-baked one around */
+    if(easy->easy_conn &&
+       (easy->easy_conn->data == easy->easy_handle)) {
 
       /* Curl_done() clears the conn->data field to lose the association
          between the easy handle and the connection */
diff --git a/lib/url.c b/lib/url.c
index 148d7b2bb..6d1fa0459 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2030,7 +2030,8 @@ ConnectionExists(struct SessionHandle *data,
                                   from the multi */
     }
 
-    infof(data, "Examining connection #%ld for reuse\n", check->connectindex);
+    DEBUGF(infof(data, "Examining connection #%ld for reuse\n",
+                 check->connectindex));
 
     if(check->inuse && !canPipeline) {
       /* can only happen within multi handles, and means that another easy
@@ -2056,11 +2057,11 @@ ConnectionExists(struct SessionHandle *data,
       continue;
     }
 
-    if (data->state.is_in_pipeline && check->bits.close) {
-        /* Don't pick a connection that is going to be closed */
-        infof(data, "Connection #%ld has been marked for close, can't reuse\n",
-              check->connectindex);
-        continue;
+    if (check->bits.close) {
+      /* Don't pick a connection that is going to be closed. */
+      infof(data, "Connection #%ld has been marked for close, can't reuse\n",
+            check->connectindex);
+      continue;
     }
 
     if((needle->protocol&PROT_SSL) != (check->protocol&PROT_SSL))
@@ -4136,8 +4137,9 @@ CURLcode Curl_async_resolved(struct connectdata *conn,
 
 
 CURLcode Curl_done(struct connectdata **connp,
-                   CURLcode status, bool premature) /* an error if this is called after an
-                                       error was detected */
+                   CURLcode status,  /* an error if this is called after an
+                                        error was detected */
+                   bool premature)
 {
   CURLcode result;
   struct connectdata *conn = *connp;