cURL / Mailing Lists / curl-library / Single Mail

curl-library

[PATCH 7/7] pipelining: keep used sockets in pipeline active.

From: Carlo Wood <carlo_at_alinoe.com>
Date: Thu, 6 Nov 2014 03:03:21 +0100

This fixes bug https://sourceforge.net/p/curl/bugs/1445/

Libcurl historically has a strict one-on-one relationship between (easy)
handles (struct SessionHandle, often called 'data') and connections
(struct connectdata, often called 'conn'). This is apparent from the fact
that data->easy_conn points to the connectdata that 'data' is using
while conn->data points back to its 'owner'.

Since pipelining this picture has changed: many easy handles can be
using a single connection. While this doesn't seem to pose a problem the
single 'easy_conn' pointer, the single 'conn->data' pointer becomes
questionable. The latter has been partially addressed by stating that
only easy handles at the head of a pipe would be 'owners', but that
still would leave two owner: one request that is at the head of the send
pipe and potentially one that is at the head of the recv pipe. Moreover,
this picture simply doesn't hold: a LOT of code depends on some kind of
relationship without which functionality breaks EVEN when the request
being handled is NOT at a pipe head. This is mostly the case for handles
that timed out (which however is also abused to process newly added
handles which might not be at the head) and for example when handles are
being closed (ie, as a result of a pipe break), etc.

After a deep investigation I determined that the following relationship
is sufficient and required: data->easy_conn->data == data. When at the
right places this relationship is ensured then the code works as
intended. For all other means one has to assume that data->easy_conn can
point to any request-- at 'random'-- and is not useful for pipelining to
be used. Instead one should look at conn->send_pipe and conn->recv_pipe,
which hold all information needed ON TOP of the request being handled
(conn->data, which does not necessarily have to be in either pipe at
all).

This commit centers around a functional bug in how singlesocket()
worked: this function is responsible for adding and removing 'active'
sockets from being watched for activity. It had two problems:
1) when data->easy_conn is set to NULL before calling this function, it
has no direct access to the connectiondata, or send_pipe/recv_pipe
thereof, and hence was not able to take the correct information into
account. The result was that socket were removed from being watched for
activity while other request(s) still existed that needed this.
2) The top part of the function intended to only add new sockets: start
watching them for either POLL_IN or POLL_OUT, but inadvertedly could
switch an action from POLL_OUT to POLL_IN while *another* request in the
send_pipe still needed the POLL_OUT.

The way singlesocket() works is by asking for the current action on the
active sockets on the handle being passed and compare that to the action
that this socket needed last time. From that it is deduced which changes
are needed for that socket. If a removal is needed (of either POLL_IN,
POLL_OUT or both) then it double checks with the send_pipe and recv_pipe
to see if that's ok: if not other requests still need that action, and
if not makes the change.

The solution used to fix problem 1) is that when a socket is
disassociated from a connection (setting easy_conn to NULL) then
immediately it is checked if other requests in the pipes require a
socket to be handled; and if that is the case to also disassociate the
handle from the socket (clearing data->socket[]). This algorithm is
implemented in the new functions Curl_disassociate_socket() and
Curl_disassociate_conn(). The latter is to be called when formerly
easy_conn was set to NULL, except when the connection was freed and/or
could be freed while the connection became unusable for all request.

Note that because of all this, a strict policy needed to be followed:
whenever a connectdata is freed, we do not want an easy_conn to still
point to it; otherwise it might still be accessed (ie, by calling
Curl_disassociate_conn()). Therefore I built in into Curl_disconnect()
that easy_conn is set to NULL when freed. That also means that it is no
longer needed to do that elsewhere (ie, immediately after returning from
Curl_disconnect or as function of some return value), ie, after calling
Curl_async_resolved(). easy_conn is now set to NULL automatically
whenever the connection is freed.

This commit fixes another bug by disassociating all handles from
disconnected sockets (in Curl_disconnect()). Before it was possible that
this same socket fd was reused by another request before old request,
still having that fd in their socket[] area, would -say- time out and
get processed - which could lead to that socket being seens as going
from active to non-active and being removed from select() while still
being used by another connnectdata struct and handles.

---
 lib/multi.c    | 200 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 lib/transfer.c |   5 +-
 lib/url.c      |  59 +++++++++++++----
 3 files changed, 227 insertions(+), 37 deletions(-)
diff --git a/lib/multi.c b/lib/multi.c
index 2b5ec20..0438f03 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -51,6 +51,9 @@
 /* The last #include file should be: */
 #include "memdebug.h"
 
+void Curl_disassociate_conn(struct SessionHandle *data, bool reset_owner);
+void Curl_disassociate_socket(struct SessionHandle *data, curl_socket_t s);
+
 /*
   CURL_SOCKET_HASH_TABLE_SIZE should be a prime number. Increasing it from 97
   to 911 takes on a 32-bit machine 4 x 804 = 3211 more bytes.  Still, every
@@ -576,10 +579,7 @@ CURLMcode curl_multi_remove_handle(CURLM *multi_handle,
                                 vanish with this handle */
 
   /* Remove the association between the connection and the handle */
-  if(data->easy_conn) {
-    data->easy_conn->data = NULL;
-    data->easy_conn = NULL;
-  }
+  Curl_disassociate_conn(data, TRUE);
 
   data->multi = NULL; /* clear the association to this multi handle */
 
@@ -623,6 +623,8 @@ bool Curl_multi_pipeline_enabled(const struct Curl_multi *multi)
 
 void Curl_multi_handlePipeBreak(struct SessionHandle *data)
 {
+  /* Just set this to NULL instead of calling Curl_disassociate_conn,
+     because if the pipe broke then the socket should be closed anyway */
   data->easy_conn = NULL;
 }
 
@@ -954,6 +956,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
       }
 
       data->state.pipe_broke = FALSE;
+      /* Set this to NULL (instead of calling Curl_disassociate_conn())
+         because if the pipe broke then the connectdata might be freed
+         and we're not allowed to access it and it's OK to close the
+         socket in any case */
       data->easy_conn = NULL;
       break;
     }
@@ -1135,11 +1141,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         data->result = Curl_async_resolved(data->easy_conn,
                                            &protocol_connect);
 
-        if(CURLE_OK != data->result)
-          /* if Curl_async_resolved() returns failure, the connection struct
-             is already freed and gone */
-          data->easy_conn = NULL;           /* no more connection */
-        else {
+        if(CURLE_OK == data->result) {
           /* call again please so that we get the next socket setup */
           result = CURLM_CALL_MULTI_PERFORM;
           if(protocol_connect)
@@ -1682,8 +1684,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
          * access free'd data, if the connection is free'd and the handle
          * removed before we perform the processing in CURLM_STATE_COMPLETED
          */
-        if(data->easy_conn)
-          data->easy_conn = NULL;
+        Curl_disassociate_conn(data, FALSE);
       }
 
       if(data->set.wildcardmatch) {
@@ -1708,7 +1709,7 @@ 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 */
-      data->easy_conn = NULL;
+      Curl_disassociate_conn(data, FALSE);
 
       Curl_expire(data, 0); /* stop all timers */
       break;
@@ -1730,6 +1731,11 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
         /* NOTE: no attempt to disconnect connections must be made
            in the case blocks above - cleanup happens only here */
 
+        /* It would be bad if this flag is set here, because
+           if it is set then data->easy_conn might point to
+           freed memory-- I don't think it CAN be set though */
+        DEBUGASSERT(!data->state.pipe_broke);
+
         data->state.pipe_broke = FALSE;
 
         if(data->easy_conn) {
@@ -1746,11 +1752,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
           if(disconnect_conn) {
             /* disconnect properly */
             Curl_disconnect(data->easy_conn, /* dead_connection */ FALSE);
-
-            /* This is where we make sure that the easy_conn pointer is reset.
-               We don't have to do this in every case block above where a
-               failure is detected */
-            data->easy_conn = NULL;
           }
         }
         else if(data->mstate == CURLM_STATE_CONNECT) {
@@ -1982,6 +1983,136 @@ CURLMsg *curl_multi_info_read(CURLM *multi_handle, int *msgs_in_queue)
 }
 
 /*
+ * Curl_disassociate_socket() disassociates the handle 'data' from
+ * the socket 's'. The reason being that we don't want singlesocket()
+ * to conclude that this socket was in use by this handle (and now
+ * not anymore), possibly causing it to stop monitoring that socket,
+ * which could be an entirely different socket by that time, or
+ * because the socket is still in use by other handles on the same
+ * pipe.
+ */
+void Curl_disassociate_socket(struct SessionHandle *data, curl_socket_t s)
+{
+  int i;
+  for(i = 0; i < data->numsocks; ++i) {
+    if(data->sockets[i] == s) {
+      data->numsocks--;
+      data->sockets[i] = data->sockets[data->numsocks];
+      break;
+    }
+  }
+}
+
+/*
+ * Curl_disassociate_conn() disassociates the handle 'data' from
+ * any connection by setting data->easy_conn to NULL.
+ *
+ * After that is done multi_getsock() will return 0,
+ * indicating that this handle is not using any sockets
+ * anymore. The result of that is that a call to singlesocket()
+ * with this handle would remove all currently associated
+ * sockets EVEN if they are still in use by other requests
+ * in the pipeline of this connection!
+ *
+ * Therefore this function has a second task: to stop that
+ * from happening.
+ */
+void Curl_disassociate_conn(struct SessionHandle *data, bool reset_owner)
+{
+  struct connectdata *conn = data->easy_conn;
+  if(!conn)
+    return; /* Not associated */
+  DEBUGASSERT(conn->data == data);
+  infof(data, "Disassociating handle %p from connection %ld\n",
+        data, conn->connection_id);
+  /* The caller might need this to stay at the current 'active' handle,
+     so only reset it when requested */
+  if(reset_owner)
+    conn->data = NULL;
+  /* And of course, disassociate the handle from this connection */
+  data->easy_conn = NULL;
+
+  /* Check if the connection has a pipeline */
+  if(conn->send_pipe) {
+    size_t nr_of_handles;
+    /* Make sure to unlink the connection between the socket and this handle */
+    curl_socket_t s = conn->sock[FIRSTSOCKET];
+    struct Curl_sh_entry *entry =
+        Curl_hash_pick(data->multi->sockhash, (char *)&s, sizeof(s));
+    if(entry) {
+      int loop = 0;
+      /* Make entry->easy point to one of the pipe heads but
+         not to data. If data is at a head, skip it and pick
+         the next one */
+      while(entry->easy == data) {
+        /* Try recv_pipe first, then send_pipe */
+        struct curl_llist *pipe = loop ? conn->send_pipe : conn->recv_pipe;
+        if(pipe->head) {
+          if(pipe->head->ptr != data) {
+            entry->easy = pipe->head->ptr;
+            break;
+          }
+          else if(pipe->head->next) {
+            entry->easy = pipe->head->next->ptr;
+            break;
+          }
+        }
+        if(++loop == 2) {
+          /* No other handles found - just let singlesocket() do its work */
+          return;
+        }
+      }
+    }
+    /* Check if there are any other handles in either pipe */
+    nr_of_handles = conn->send_pipe->size + conn->recv_pipe->size;
+    if(nr_of_handles > 1 ||
+       (nr_of_handles == 1 &&
+        !(conn->send_pipe->head && conn->send_pipe->head->ptr == data) &&
+        !(conn->recv_pipe->head && conn->recv_pipe->head->ptr == data))) {
+      /* Other handles exist!
+         Remove the socket that is used by the connection from the
+         used sockets array of this handle, so it won't notice that
+         it doesn't use it anymore the next call to singlesocket */
+      /* It's suffienct to only compare with FIRSTSOCKET because
+         pipelining is only used by HTTP, which only uses one socket */
+      Curl_disassociate_socket(data, s);
+    }
+  }
+}
+
+/* Return true if, besides data, there is at least one other handle in pipe */
+static bool additional_handle_in_pipe(struct curl_llist *pipe,
+                                      struct SessionHandle *data)
+{
+  return (pipe &&
+          !(pipe->size == 0 ||
+            (pipe->size == 1 && pipe->head->ptr == data)));
+}
+
+#ifdef DEBUGBUILD
+static void socket_removal_sanity_check(struct SessionHandle *data,
+                                        struct Curl_sh_entry *entry,
+                                        unsigned int action_removal)
+{
+  if((action_removal & CURL_POLL_INOUT)) {
+    /* Find a connection */
+    struct connectdata *conn = data->easy_conn;
+    if(!conn) {
+      conn = entry->easy->easy_conn;
+    }
+    if(conn) {
+      /* Make sure there isn't anything else in
+         the pipe that has its action removed */
+      DEBUGASSERT(!(action_removal & CURL_POLL_OUT) ||
+                  !additional_handle_in_pipe(conn->send_pipe, data));
+      DEBUGASSERT(!(action_removal & CURL_POLL_IN) ||
+                  !additional_handle_in_pipe(conn->recv_pipe, data));
+    }
+  }
+}
+#endif
+
+/*
  * singlesocket() checks what sockets we deal with and their "action state"
  * and if we have a different state in any of those sockets from last time we
  * call the callback accordingly.
@@ -1996,6 +2127,7 @@ static void singlesocket(struct Curl_multi *multi,
   int num;
   unsigned int curraction;
   bool remove_sock_from_hash;
+  struct connectdata *easy_conn = data->easy_conn;
 
   for(i=0; i< MAX_SOCKSPEREASYHANDLE; i++)
     socks[i] = CURL_SOCKET_BAD;
@@ -2019,9 +2151,17 @@ static void singlesocket(struct Curl_multi *multi,
     /* get it from the hash */
     entry = Curl_hash_pick(multi->sockhash, (char *)&s, sizeof(s));
 
-    if(curraction & GETSOCK_READSOCK(i))
+    /*
+     * Construct a new action for socket s.
+     * If this handle has an easy_conn and an additional handle is
+     * found in its send/recv pipe then we are pipelining and
+     * the additional handle requires us to keep polling the socket.
+     */
+    if((curraction & GETSOCK_READSOCK(i)) ||
+       (easy_conn && additional_handle_in_pipe(easy_conn->recv_pipe, data)))
       action |= CURL_POLL_IN;
-    if(curraction & GETSOCK_WRITESOCK(i))
+    if((curraction & GETSOCK_WRITESOCK(i)) ||
+       (easy_conn && additional_handle_in_pipe(easy_conn->send_pipe, data)))
       action |= CURL_POLL_OUT;
 
     if(entry) {
@@ -2038,6 +2178,11 @@ static void singlesocket(struct Curl_multi *multi,
         return;
     }
 
+#ifdef DEBUGBUILD
+    /* Sanity check. Are we removing an action? */
+    socket_removal_sanity_check(data, entry, (entry->action & ~action));
+#endif
+
     /* we know (entry != NULL) at this point, see the logic above */
     if(multi->socket_cb)
       multi->socket_cb(data,
@@ -2073,9 +2218,8 @@ static void singlesocket(struct Curl_multi *multi,
         /* check if the socket to be removed serves a connection which has
            other easy-s in a pipeline. In this case the socket should not be
            removed. */
-        struct connectdata *easy_conn = data->easy_conn;
         if(easy_conn) {
-          if(easy_conn->recv_pipe && easy_conn->recv_pipe->size > 1) {
+          if(additional_handle_in_pipe(easy_conn->recv_pipe, data)) {
             /* the handle should not be removed from the pipe yet */
             remove_sock_from_hash = FALSE;
 
@@ -2089,7 +2233,7 @@ static void singlesocket(struct Curl_multi *multi,
                 entry->easy = easy_conn->recv_pipe->head->ptr;
             }
           }
-          if(easy_conn->send_pipe  && easy_conn->send_pipe->size > 1) {
+          if(additional_handle_in_pipe(easy_conn->send_pipe, data)) {
             /* the handle should not be removed from the pipe yet */
             remove_sock_from_hash = FALSE;
 
@@ -2110,13 +2254,16 @@ static void singlesocket(struct Curl_multi *multi,
         }
       }
       else
-        /* just a precaution, this socket really SHOULD be in the hash already
-           but in case it isn't, we don't have to tell the app to remove it
-           either since it never got to know about it */
+        /* If the socket is not in the hash then it was already removed
+           before by a call to Curl_multi_closed(). For example as a result
+           of calling Curl_disconnect() called from Curl_done() */
         remove_sock_from_hash = FALSE;
 
       if(remove_sock_from_hash) {
         /* in this case 'entry' is always non-NULL */
+#ifdef DEBUGBUILD
+        socket_removal_sanity_check(data, entry, entry->action);
+#endif
         if(multi->socket_cb)
           multi->socket_cb(data,
                            s,
@@ -2153,6 +2300,9 @@ void Curl_multi_closed(struct connectdata *conn, curl_socket_t s)
       Curl_hash_pick(multi->sockhash, (char *)&s, sizeof(s));
 
     if(entry) {
+#ifdef DEBUGBUILD
+      socket_removal_sanity_check(conn->data, entry, entry->action);
+#endif
       if(multi->socket_cb)
         multi->socket_cb(conn->data, s, CURL_POLL_REMOVE,
                          multi->socket_userp,
diff --git a/lib/transfer.c b/lib/transfer.c
index 3def3fa..ce45da0 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -87,6 +87,9 @@
 /* The last #include file should be: */
 #include "memdebug.h"
 
+/* Defined in multi.c */
+void Curl_disassociate_conn(struct SessionHandle*, bool reset_owner);
+
 /*
  * This function will call the read callback to fill our buffer with data
  * to upload.
@@ -1837,7 +1840,7 @@ Curl_reconnect_request(struct connectdata **connp)
      parent functions -- note that Curl_done above might already have
      done this for us. */
   if(data->easy_conn)
-    data->easy_conn = NULL;
+    Curl_disassociate_conn(data, FALSE);
 
   /*
    * According to bug report #1330310. We need to check for CURLE_SEND_ERROR
diff --git a/lib/url.c b/lib/url.c
index 9fa7228..671468a 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -134,6 +134,10 @@ int curl_win32_idn_to_ascii(const char *in, char **out);
 /* The last #include file should be: */
 #include "memdebug.h"
 
+/* Defined in multi.c */
+void Curl_disassociate_conn(struct SessionHandle*, bool reset_owner);
+void Curl_disassociate_socket(struct SessionHandle *data, curl_socket_t s);
+
 /* Local static prototypes */
 static struct connectdata *
 find_oldest_idle_connection(struct SessionHandle *data);
@@ -141,7 +145,8 @@ static struct connectdata *
 find_oldest_idle_connection_in_bundle(struct SessionHandle *data,
                                       struct connectbundle *bundle);
 static void conn_free(struct connectdata *conn);
-static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke);
+static void signalPipeClose(struct curl_llist *pipeline,
+                            curl_socket_t s, bool pipe_broke);
 static CURLcode do_init(struct connectdata *conn);
 static CURLcode parse_url_login(struct SessionHandle *data,
                                 struct connectdata *conn,
@@ -2647,15 +2652,27 @@ static void conn_free(struct connectdata *conn)
 CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
 {
   struct SessionHandle *data;
+  curl_socket_t s;
+
   if(!conn)
     return CURLE_OK; /* this is closed and fine already */
+
   data = conn->data;
+  s = conn->sock[FIRSTSOCKET];
 
   if(!data) {
     DEBUGF(fprintf(stderr, "DISCONNECT without easy handle, ignoring\n"));
     return CURLE_OK;
   }
 
+  /* Make sure that singlesocket() will never stop handling a socket
+     with fd conn->sock[FIRSTSOCKET] somewhere in the future: it would
+     be a completely new connection that reused the fd! */
+  Curl_disassociate_socket(data, s);
+
+  DEBUGASSERT(!data->easy_conn || data->easy_conn == conn);
+  data->easy_conn = NULL;
+
   if(conn->dns_entry != NULL) {
     Curl_resolv_unlock(data, conn->dns_entry);
     conn->dns_entry = NULL;
@@ -2699,8 +2716,8 @@ CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
 
   /* Indicate to all handles on the pipe that we're dead */
   if(Curl_multi_pipeline_enabled(data->multi)) {
-    signalPipeClose(conn->send_pipe, TRUE);
-    signalPipeClose(conn->recv_pipe, TRUE);
+    signalPipeClose(conn->send_pipe, s, TRUE);
+    signalPipeClose(conn->recv_pipe, s, TRUE);
   }
 
   conn_free(conn);
@@ -2810,7 +2827,8 @@ void Curl_getoff_all_pipelines(struct SessionHandle *data,
     conn->writechannel_inuse = FALSE;
 }
 
-static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke)
+static void signalPipeClose(struct curl_llist *pipeline,
+                            curl_socket_t s, bool pipe_broke)
 {
   struct curl_llist_element *curr;
 
@@ -2831,6 +2849,7 @@ static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke)
 
     if(pipe_broke)
       data->state.pipe_broke = TRUE;
+    Curl_disassociate_socket(data, s);
     Curl_multi_handlePipeBreak(data);
     Curl_llist_remove(pipeline, curr, NULL);
     curr = next;
@@ -2956,6 +2975,7 @@ static bool disconnect_if_dead(struct connectdata *conn,
       infof(data, "Connection %ld seems to be dead!\n", conn->connection_id);
 
       /* disconnect resources */
+      data->easy_conn = conn;
       Curl_disconnect(conn, /* dead_connection */TRUE);
       return TRUE;
     }
@@ -3303,7 +3323,9 @@ ConnectionDone(struct SessionHandle *data, struct connectdata *conn)
       conn_candidate->data = data;
 
       /* the winner gets the honour of being disconnected */
+      data->easy_conn = conn_candidate;
       (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
+      /* Now data->easy_conn == NULL */
     }
   }
 
@@ -5528,6 +5550,8 @@ static CURLcode create_conn(struct SessionHandle *data,
   }
 
   prune_dead_connections(data);
+  /* prune_dead_connections messes with easy_conn, so set it again */
+  *in_connect = conn;
 
   /*************************************************************
    * Check the current list of connections to see if we can
@@ -5541,8 +5565,12 @@ static CURLcode create_conn(struct SessionHandle *data,
      authentication phase). */
   if(data->set.reuse_fresh && !data->state.this_is_a_follow)
     reuse = FALSE;
-  else
+  else {
     reuse = ConnectionExists(data, conn, &conn_temp, &force_reuse);
+    /* ConnectionExists calls disconnect_if_dead which messes
+       with data->easy_conn. Set it again... */
+    *in_connect = conn;
+  }
 
   /* If we found a reusable connection, we may still want to
      open a new connection if we are pipelining. */
@@ -5603,6 +5631,7 @@ static CURLcode create_conn(struct SessionHandle *data,
         /* Set the connection's owner correctly, then kill it */
         conn_candidate->data = data;
         (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
+        *in_connect = conn;     /* Was reset by Curl_disconnect */
       }
       else
         no_connections_available = TRUE;
@@ -5619,6 +5648,7 @@ static CURLcode create_conn(struct SessionHandle *data,
         /* Set the connection's owner correctly, then kill it */
         conn_candidate->data = data;
         (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
+        *in_connect = conn;     /* Was reset by Curl_disconnect */
       }
       else
         no_connections_available = TRUE;
@@ -5800,6 +5830,9 @@ CURLcode Curl_connect(struct SessionHandle *data,
      the value on error without worries, if anything goes wrong */
   DEBUGASSERT(*in_connect == NULL);
 
+  /* Required correct ownership */
+  DEBUGASSERT(in_connect == &data->easy_conn);
+
   /* call the stuff that needs to be called */
   result = create_conn(data, in_connect, asyncp);
 
@@ -5828,8 +5861,8 @@ CURLcode Curl_connect(struct SessionHandle *data,
   if(result && *in_connect) {
     /* We're not allowed to return failure with memory left allocated
        in the connectdata struct, free those here */
+    (*in_connect)->data = data;
     Curl_disconnect(*in_connect, FALSE); /* close the connection */
-    *in_connect = NULL;           /* return a NULL */
   }
 
   return result;
@@ -5952,7 +5985,8 @@ CURLcode Curl_done(struct connectdata **connp,
       data->state.lastconnect = NULL;
   }
 
-  /* This is true because we only get here from Curl_reconnect_request
+  /*
+   * This is true because we only get here from Curl_reconnect_request
    * where the exact same assert holds, or from curl_multi_remove_handle
    * which only calls Curl_done when data->easy_conn->data == data, or from
    * multi_runsingle with various states (PROTOCONNECT, DO, DOING, DO_MORE,
@@ -5962,10 +5996,13 @@ CURLcode Curl_done(struct connectdata **connp,
    */
   DEBUGASSERT(connp == &data->easy_conn);
 
-  data->easy_conn = NULL; /* to make the caller of this function better detect
-                             that this was either closed or handed over to the
-                             connection cache here, and therefore cannot be
-                             used from this point on */
+  /*
+   * Call Curl_disassociate_conn() to make the caller of this function better
+   * detect that this was either closed or handed over to the connection
+   * cache here, and therefore cannot be used from this point on
+   */
+  Curl_disassociate_conn(data, FALSE);
+
   Curl_free_request_state(data);
 
   return result;
-- 
2.1.1
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2014-11-06