[GNUnet-SVN] [gnurl] 04/153: conn: remove the boolean 'inuse' field

[gnunet]

This is an automated email from the git hooks/post-receive script.

ng0 pushed a commit to branch master
in repository gnurl.

commit 1b76c38904f0033ac1403ec4b31c28f23805c0d4
Author: Daniel Stenberg <address@hidden>
AuthorDate: Wed Jul 4 00:55:48 2018 +0200

    conn: remove the boolean 'inuse' field
    
    ... as the usage needs to be counted.
---
 lib/conncache.c     | 30 ++++++++++++----------------
 lib/hostasyn.c      |  4 ++--
 lib/multi.c         | 48 +++++++++++++++++++++++++--------------------
 lib/url.c           | 56 ++++++++++++++++++++++++-----------------------------
 lib/url.h           |  3 ++-
 lib/urldata.h       | 11 ++++++-----
 tests/data/test1554 |  6 ------
 7 files changed, 74 insertions(+), 84 deletions(-)

diff --git a/lib/conncache.c b/lib/conncache.c
index 76428eb86..957ced3c7 100644
--- a/lib/conncache.c
+++ b/lib/conncache.c
@@ -63,10 +63,9 @@
 
 static void conn_llist_dtor(void *user, void *element)
 {
-  struct connectdata *data = element;
+  struct connectdata *conn = element;
   (void)user;
-
-  data->bundle = NULL;
+  conn->bundle = NULL;
 }
 
 static CURLcode bundle_create(struct Curl_easy *data,
@@ -313,19 +312,20 @@ void Curl_conncache_remove_conn(struct connectdata *conn, 
bool lock)
      due to a failed connection attempt, before being added to a bundle */
   if(bundle) {
     if(lock) {
-      CONN_LOCK(conn->data);
+      CONN_LOCK(data);
     }
+    conn->data = NULL; /* detach */
     bundle_remove_conn(bundle, conn);
     if(bundle->num_connections == 0)
       conncache_remove_bundle(connc, bundle);
     conn->bundle = NULL; /* removed from it */
     if(connc) {
       connc->num_conn--;
-      DEBUGF(infof(conn->data, "The cache now contains %zu members\n",
+      DEBUGF(infof(data, "The cache now contains %zu members\n",
                    connc->num_conn));
     }
     if(lock) {
-      CONN_UNLOCK(conn->data);
+      CONN_UNLOCK(data);
     }
   }
 }
@@ -433,19 +433,11 @@ bool Curl_conncache_return_conn(struct connectdata *conn)
     infof(data, "Connection cache is full, closing the oldest one.\n");
 
     conn_candidate = Curl_conncache_extract_oldest(data);
-
     if(conn_candidate) {
-      /* Set the connection's owner correctly */
-      conn_candidate->data = data;
-
       /* the winner gets the honour of being disconnected */
-      (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
+      (void)Curl_disconnect(data, conn_candidate, /* dead_connection */ FALSE);
     }
   }
-  CONN_LOCK(data);
-  conn->inuse = FALSE; /* Mark the connection unused */
-  conn->data = NULL; /* no owner */
-  CONN_UNLOCK(data);
 
   return (conn_candidate == conn) ? FALSE : TRUE;
 
@@ -479,7 +471,7 @@ Curl_conncache_extract_bundle(struct Curl_easy *data,
   while(curr) {
     conn = curr->ptr;
 
-    if(!conn->inuse) {
+    if(!CONN_INUSE(conn)) {
       /* Set higher score for the age passed since the connection was used */
       score = Curl_timediff(now, conn->now);
 
@@ -496,6 +488,7 @@ Curl_conncache_extract_bundle(struct Curl_easy *data,
     data->state.conn_cache->num_conn--;
     DEBUGF(infof(data, "The cache now contains %zu members\n",
                  data->state.conn_cache->num_conn));
+    conn_candidate->data = data; /* associate! */
   }
 
   return conn_candidate;
@@ -536,7 +529,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
     while(curr) {
       conn = curr->ptr;
 
-      if(!conn->inuse) {
+      if(!CONN_INUSE(conn)) {
         /* Set higher score for the age passed since the connection was used */
         score = Curl_timediff(now, conn->now);
 
@@ -557,6 +550,7 @@ Curl_conncache_extract_oldest(struct Curl_easy *data)
     connc->num_conn--;
     DEBUGF(infof(data, "The cache now contains %zu members\n",
                  connc->num_conn));
+    conn_candidate->data = data; /* associate! */
   }
   CONN_UNLOCK(data);
 
@@ -577,7 +571,7 @@ void Curl_conncache_close_all_connections(struct conncache 
*connc)
                                      pointer */
     /* This will remove the connection from the cache */
     connclose(conn, "kill all");
-    (void)Curl_disconnect(conn, FALSE);
+    (void)Curl_disconnect(connc->closure_handle, conn, FALSE);
     sigpipe_restore(&pipe_st);
 
     conn = Curl_conncache_find_first_connection(connc);
diff --git a/lib/hostasyn.c b/lib/hostasyn.c
index 7b6e8568a..35b52a90c 100644
--- a/lib/hostasyn.c
+++ b/lib/hostasyn.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2015, Daniel Stenberg, <address@hidden>, et al.
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <address@hidden>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -131,7 +131,7 @@ CURLcode Curl_async_resolved(struct connectdata *conn,
   if(result)
     /* We're not allowed to return failure with memory left allocated
        in the connectdata struct, free those here */
-    Curl_disconnect(conn, FALSE); /* close the connection */
+    Curl_disconnect(conn->data, conn, FALSE); /* close the connection */
 
   return result;
 }
diff --git a/lib/multi.c b/lib/multi.c
index c1d48a3ed..decdbc443 100644
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -604,7 +604,7 @@ static CURLcode multi_done(struct connectdata **connp,
 #endif
      ) || conn->bits.close
        || (premature && !(conn->handler->flags & PROTOPT_STREAM))) {
-    CURLcode res2 = Curl_disconnect(conn, premature); /* close connection */
+    CURLcode res2 = Curl_disconnect(data, conn, premature);
 
     /* If we had an error already, make sure we return that one. But
        if we got a new error, return that. */
@@ -622,7 +622,7 @@ static CURLcode multi_done(struct connectdata **connp,
              conn->bits.conn_to_host ? conn->conn_to_host.dispname :
              conn->host.dispname);
 
-    /* the connection is no longer in use */
+    /* the connection is no longer in use by this transfer */
     if(Curl_conncache_return_conn(conn)) {
       /* remember the most recently used connection */
       data->state.lastconnect = conn;
@@ -2109,7 +2109,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
             /* Don't attempt to send data over a connection that timed out */
             bool dead_connection = result == CURLE_OPERATION_TIMEDOUT;
             /* disconnect properly */
-            Curl_disconnect(data->easy_conn, dead_connection);
+            Curl_disconnect(data, data->easy_conn, dead_connection);
 
             /* 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
@@ -2471,20 +2471,23 @@ void Curl_updatesocket(struct Curl_easy *data)
 
 void Curl_multi_closed(struct connectdata *conn, curl_socket_t s)
 {
-  struct Curl_multi *multi = conn->data->multi;
-  if(multi) {
-    /* this is set if this connection is part of a handle that is added to
-       a multi handle, and only then this is necessary */
-    struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);
-
-    if(entry) {
-      if(multi->socket_cb)
-        multi->socket_cb(conn->data, s, CURL_POLL_REMOVE,
-                         multi->socket_userp,
-                         entry->socketp);
+  if(conn->data) {
+    /* if there's still an easy handle associated with this connection */
+    struct Curl_multi *multi = conn->data->multi;
+    if(multi) {
+      /* this is set if this connection is part of a handle that is added to
+         a multi handle, and only then this is necessary */
+      struct Curl_sh_entry *entry = sh_getentry(&multi->sockhash, s);
+
+      if(entry) {
+        if(multi->socket_cb)
+          multi->socket_cb(conn->data, s, CURL_POLL_REMOVE,
+                           multi->socket_userp,
+                           entry->socketp);
 
-      /* now remove it from the socket hash */
-      sh_delentry(&multi->sockhash, s);
+        /* now remove it from the socket hash */
+        sh_delentry(&multi->sockhash, s);
+      }
     }
   }
 }
@@ -3135,12 +3138,15 @@ static void process_pending_handles(struct Curl_multi 
*multi)
   }
 }
 
-void Curl_set_in_callback(struct Curl_easy *easy, bool value)
+void Curl_set_in_callback(struct Curl_easy *data, bool value)
 {
-  if(easy->multi_easy)
-    easy->multi_easy->in_callback = value;
-  else if(easy->multi)
-      easy->multi->in_callback = value;
+  /* might get called when there is no data pointer! */
+  if(data) {
+    if(data->multi_easy)
+      data->multi_easy->in_callback = value;
+    else if(data->multi)
+      data->multi->in_callback = value;
+  }
 }
 
 bool Curl_is_in_callback(struct Curl_easy *easy)
diff --git a/lib/url.c b/lib/url.c
index 27b2c1e14..b81174497 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -734,17 +734,20 @@ static void conn_free(struct connectdata *conn)
  * primary connection, like when freeing room in the connection cache or
  * killing of a dead old connection.
  *
+ * A connection needs an easy handle when closing down. We support this passed
+ * in separately since the connection to get closed here is often already
+ * disconnected from an easy handle.
+ *
  * This function MUST NOT reset state in the Curl_easy struct if that
  * isn't strictly bound to the life-time of *this* particular connection.
  *
  */
 
-CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
+CURLcode Curl_disconnect(struct Curl_easy *data,
+                         struct connectdata *conn, bool dead_connection)
 {
-  struct Curl_easy *data;
   if(!conn)
     return CURLE_OK; /* this is closed and fine already */
-  data = conn->data;
 
   if(!data) {
     DEBUGF(fprintf(stderr, "DISCONNECT without easy handle, ignoring\n"));
@@ -755,13 +758,13 @@ CURLcode Curl_disconnect(struct connectdata *conn, bool 
dead_connection)
    * If this connection isn't marked to force-close, leave it open if there
    * are other users of it
    */
-  if(!conn->bits.close &&
-     (conn->send_pipe.size + conn->recv_pipe.size)) {
-    DEBUGF(infof(data, "Curl_disconnect, usecounter: %zu\n",
-                 conn->send_pipe.size + conn->recv_pipe.size));
+  if(!conn->bits.close && CONN_INUSE(conn)) {
+    DEBUGF(fprintf(stderr, "Curl_disconnect when inuse: %d\n",
+                   CONN_INUSE(conn)));
     return CURLE_OK;
   }
 
+  conn->data = data;
   if(conn->dns_entry != NULL) {
     Curl_resolv_unlock(data, conn->dns_entry);
     conn->dns_entry = NULL;
@@ -959,7 +962,7 @@ static bool extract_if_dead(struct connectdata *conn,
                             struct Curl_easy *data)
 {
   size_t pipeLen = conn->send_pipe.size + conn->recv_pipe.size;
-  if(!pipeLen && !conn->inuse) {
+  if(!pipeLen && !CONN_INUSE(conn)) {
     /* The check for a dead socket makes sense only if there are no
        handles in pipeline and the connection isn't already marked in
        use */
@@ -1025,7 +1028,7 @@ static void prune_dead_connections(struct Curl_easy *data)
     while(Curl_conncache_foreach(data, data->state.conn_cache, &prune,
                                  call_extract_if_dead)) {
       /* disconnect it */
-      (void)Curl_disconnect(prune.extracted, /* dead_connection */TRUE);
+      (void)Curl_disconnect(data, prune.extracted, /* dead_connection */TRUE);
     }
     data->state.conn_cache->last_cleanup = now;
   }
@@ -1139,7 +1142,7 @@ ConnectionExists(struct Curl_easy *data,
 
       if(extract_if_dead(check, data)) {
         /* disconnect it */
-        (void)Curl_disconnect(check, /* dead_connection */TRUE);
+        (void)Curl_disconnect(data, check, /* dead_connection */TRUE);
         continue;
       }
 
@@ -1267,12 +1270,12 @@ ConnectionExists(struct Curl_easy *data,
         }
       }
 
-      if(!canpipe && check->inuse)
+      if(!canpipe && CONN_INUSE(check))
         /* this request can't be pipelined but the checked connection is
            already in use so we skip it */
         continue;
 
-      if((check->inuse) && (check->data->multi != needle->data->multi))
+      if(CONN_INUSE(check) && (check->data->multi != needle->data->multi))
         /* this could be subject for pipeline/multiplex use, but only
            if they belong to the same multi handle */
         continue;
@@ -1464,7 +1467,6 @@ ConnectionExists(struct Curl_easy *data,
 
   if(chosen) {
     /* mark it as used before releasing the lock */
-    chosen->inuse = TRUE;
     chosen->data = data; /* own it! */
     Curl_conncache_unlock(needle);
     *usethis = chosen;
@@ -4481,11 +4483,9 @@ static CURLcode create_conn(struct Curl_easy *data,
         conn_candidate = Curl_conncache_extract_bundle(data, bundle);
         Curl_conncache_unlock(conn);
 
-        if(conn_candidate) {
-          /* Set the connection's owner correctly, then kill it */
-          conn_candidate->data = data;
-          (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
-        }
+        if(conn_candidate)
+          (void)Curl_disconnect(data, conn_candidate,
+                                /* dead_connection */ FALSE);
         else {
           infof(data, "No more connections allowed to host: %zu\n",
                 max_host_connections);
@@ -4504,12 +4504,9 @@ static CURLcode create_conn(struct Curl_easy *data,
 
       /* The cache is full. Let's see if we can kill a connection. */
       conn_candidate = Curl_conncache_extract_oldest(data);
-
-      if(conn_candidate) {
-        /* Set the connection's owner correctly, then kill it */
-        conn_candidate->data = data;
-        (void)Curl_disconnect(conn_candidate, /* dead_connection */ FALSE);
-      }
+      if(conn_candidate)
+        (void)Curl_disconnect(data, conn_candidate,
+                              /* dead_connection */ FALSE);
       else {
         infof(data, "No connections available in cache\n");
         connections_available = FALSE;
@@ -4526,9 +4523,6 @@ static CURLcode create_conn(struct Curl_easy *data,
       goto out;
     }
     else {
-      /* Mark the connection as used, before we add it */
-      conn->inuse = TRUE;
-
       /*
        * This is a brand new connection, so let's store it in the connection
        * cache of ours!
@@ -4693,10 +4687,10 @@ CURLcode Curl_connect(struct Curl_easy *data,
   }
 
   if(result && *in_connect) {
-    /* We're not allowed to return failure with memory left allocated
-       in the connectdata struct, free those here */
-    Curl_disconnect(*in_connect, FALSE); /* close the connection */
-    *in_connect = NULL;           /* return a NULL */
+    /* We're not allowed to return failure with memory left allocated in the
+       connectdata struct, free those here */
+    Curl_disconnect(data, *in_connect, FALSE);
+    *in_connect = NULL; /* return a NULL */
   }
 
   return result;
diff --git a/lib/url.h b/lib/url.h
index a70bd5466..ef3ebf03e 100644
--- a/lib/url.h
+++ b/lib/url.h
@@ -39,7 +39,8 @@ void Curl_freeset(struct Curl_easy * data);
 CURLcode Curl_close(struct Curl_easy *data); /* opposite of curl_open() */
 CURLcode Curl_connect(struct Curl_easy *, struct connectdata **,
                       bool *async, bool *protocol_connect);
-CURLcode Curl_disconnect(struct connectdata *, bool dead_connection);
+CURLcode Curl_disconnect(struct Curl_easy *data,
+                         struct connectdata *, bool dead_connection);
 CURLcode Curl_protocol_connect(struct connectdata *conn, bool *done);
 CURLcode Curl_protocol_connecting(struct connectdata *conn, bool *done);
 CURLcode Curl_protocol_doing(struct connectdata *conn, bool *done);
diff --git a/lib/urldata.h b/lib/urldata.h
index 666981c38..07eb48869 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -781,11 +781,12 @@ struct connectdata {
   curl_closesocket_callback fclosesocket; /* function closing the socket(s) */
   void *closesocket_client;
 
-  bool inuse; /* This is a marker for the connection cache logic. If this is
-                 TRUE this handle is being used by one or more easy handles
-                 and can only used by any other easy handle without careful
-                 consideration (== only for pipelining/multiplexing) and it
-                 cannot be used by another multi handle! */
+  /* This is used by the connection cache logic. If this returns TRUE, this
+     handle is being used by one or more easy handles and can only used by any
+     other easy handle without careful consideration (== only for
+     pipelining/multiplexing) and it cannot be used by another multi
+     handle! */
+#define CONN_INUSE(c) ((c)->send_pipe.size || (c)->recv_pipe.size)
 
   /**** Fields set when inited and not modified again */
   long connection_id; /* Contains a unique number to make it easier to
diff --git a/tests/data/test1554 b/tests/data/test1554
index 06f189724..be48e02eb 100644
--- a/tests/data/test1554
+++ b/tests/data/test1554
@@ -38,8 +38,6 @@ run 1: foobar and so on fun!
 <- Mutex unlock
 -> Mutex lock
 <- Mutex unlock
--> Mutex lock
-<- Mutex unlock
 run 1: foobar and so on fun!
 -> Mutex lock
 <- Mutex unlock
@@ -49,8 +47,6 @@ run 1: foobar and so on fun!
 <- Mutex unlock
 -> Mutex lock
 <- Mutex unlock
--> Mutex lock
-<- Mutex unlock
 run 1: foobar and so on fun!
 -> Mutex lock
 <- Mutex unlock
@@ -58,8 +54,6 @@ run 1: foobar and so on fun!
 <- Mutex unlock
 -> Mutex lock
 <- Mutex unlock
--> Mutex lock
-<- Mutex unlock
 </datacheck>
 </reply>
 

-- 
To stop receiving notification emails like this one, please contact
address@hidden