disconnect: separate connections and easy handles better

Do not assume/store assocation between a given easy handle and the
connection if it can be avoided.

Long-term, the 'conn->data' pointer should probably be removed as it is a
little too error-prone. Still used very widely though.

Reported-by: masbug on github
Fixes #3391
Closes #3400

lib/conncache.c

@@ -302,9 +302,14 @@ CURLcode Curl_conncache_add_conn(struct conncache *connc,
   return result;
 }
 
-void Curl_conncache_remove_conn(struct connectdata *conn, bool lock)
+/*
+ * Removes the connectdata object from the connection cache *and* clears the
+ * ->data pointer association. Pass TRUE/FALSE in the 'lock' argument
+ * depending on if the parent function already holds the lock or not.
+ */
+void Curl_conncache_remove_conn(struct Curl_easy *data,
+                                struct connectdata *conn, bool lock)
 {
-  struct Curl_easy *data = conn->data;
   struct connectbundle *bundle = conn->bundle;
   struct conncache *connc = data->state.conn_cache;
 
@@ -323,6 +328,7 @@ void Curl_conncache_remove_conn(struct connectdata *conn, bool lock)
       DEBUGF(infof(data, "The cache now contains %zu members\n",
                    connc->num_conn));
     }
+    conn->data = NULL; /* clear the association */
     if(lock) {
       CONN_UNLOCK(data);
     }

lib/conncache.h

@@ -64,7 +64,8 @@ size_t Curl_conncache_bundle_size(struct connectdata *conn);
 bool Curl_conncache_return_conn(struct connectdata *conn);
 CURLcode Curl_conncache_add_conn(struct conncache *connc,
                                  struct connectdata *conn) WARN_UNUSED_RESULT;
-void Curl_conncache_remove_conn(struct connectdata *conn,
+void Curl_conncache_remove_conn(struct Curl_easy *data,
+                                struct connectdata *conn,
                                 bool lock);
 bool Curl_conncache_foreach(struct Curl_easy *data,
                             struct conncache *connc,

lib/multi.c

@@ -761,10 +761,8 @@ CURLMcode curl_multi_remove_handle(struct Curl_multi *multi,
                                 vanish with this handle */
 
   /* Remove the association between the connection and the handle */
-  if(data->easy_conn) {
-    data->easy_conn->data = NULL;
+  if(data->easy_conn)
     data->easy_conn = NULL;
-  }
 
 #ifdef USE_LIBPSL
   /* Remove the PSL association. */

lib/url.c

@@ -769,7 +769,6 @@ CURLcode Curl_disconnect(struct Curl_easy *data,
     return CURLE_OK;
   }
 
-  conn->data = data;
   if(conn->dns_entry != NULL) {
     Curl_resolv_unlock(data, conn->dns_entry);
     conn->dns_entry = NULL;
@@ -788,14 +787,13 @@ CURLcode Curl_disconnect(struct Curl_easy *data,
 
     /* unlink ourselves! */
   infof(data, "Closing connection %ld\n", conn->connection_id);
-  Curl_conncache_remove_conn(conn, TRUE);
+  Curl_conncache_remove_conn(data, conn, TRUE);
 
   free_idnconverted_hostname(&conn->host);
   free_idnconverted_hostname(&conn->conn_to_host);
   free_idnconverted_hostname(&conn->http_proxy.host);
   free_idnconverted_hostname(&conn->socks_proxy.host);
 
-  DEBUGASSERT(conn->data == data);
   /* this assumes that the pointer is still there after the connection was
      detected from the cache */
   Curl_ssl_close(conn, FIRSTSOCKET);
@@ -960,8 +958,6 @@ static bool extract_if_dead(struct connectdata *conn,
        handles in pipeline and the connection isn't already marked in
        use */
     bool dead;
-
-    conn->data = data;
     if(conn->handler->connection_check) {
       /* The protocol has a special method for checking the state of the
          connection. Use it to check if the connection is dead. */
@@ -977,8 +973,7 @@ static bool extract_if_dead(struct connectdata *conn,
 
     if(dead) {
       infof(data, "Connection %ld seems to be dead!\n", conn->connection_id);
-      Curl_conncache_remove_conn(conn, FALSE);
-      conn->data = NULL; /* detach */
+      Curl_conncache_remove_conn(data, conn, FALSE);
       return TRUE;
     }
   }