CURLMOPT_MAXCONNECTS: restore functionality

When a connection is no longer used, it is kept in the cache. If the
cache is full, the oldest idle connection is closed. If no connection is
idle, the current one is closed instead.

5 files changed, 326 insertions, 13 deletions

diff --git a/lib/url.c b/lib/url.c
index 80c8a997e..918ce58c3 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -122,6 +122,7 @@ int curl_win32_idn_to_ascii(const char *in, char **out);
 #include "http_proxy.h"
 #include "bundles.h"
 #include "conncache.h"
+#include "multihandle.h"
 
 #define _MPRINTF_REPLACE /* use our functions only */
 #include <curl/mprintf.h>
@@ -131,6 +132,8 @@ int curl_win32_idn_to_ascii(const char *in, char **out);
 #include "memdebug.h"
 
 /* Local static prototypes */
+static struct connectdata *
+find_oldest_idle_connection(struct SessionHandle *data);
 static void conn_free(struct connectdata *conn);
 static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke);
 static CURLcode do_init(struct connectdata *conn);
@@ -2710,6 +2713,57 @@ static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke)
   }
 }
 
+/*
+ * This function kills and removes an existing connection in the connection
+ * cache. The connection that has been unused for the longest time.
+ *
+ * Returns the pointer to the oldest idle connection, or NULL if none was
+ * found.
+ */
+static struct connectdata *
+find_oldest_idle_connection(struct SessionHandle *data)
+{
+  struct conncache *bc = data->state.conn_cache;
+  struct curl_hash_iterator iter;
+  struct curl_llist_element *curr;
+  struct curl_hash_element *he;
+  long highscore=-1;
+  long score;
+  struct timeval now;
+  struct connectdata *conn_candidate = NULL;
+  struct connectbundle *bundle;
+
+  now = Curl_tvnow();
+
+  Curl_hash_start_iterate(bc->hash, &iter);
+
+  he = Curl_hash_next_element(&iter);
+  while(he) {
+    struct connectdata *conn;
+
+    bundle = he->ptr;
+
+    curr = bundle->conn_list->head;
+    while(curr) {
+      conn = curr->ptr;
+
+      if(!conn->inuse) {
+        /* Set higher score for the age passed since the connection was used */
+        score = Curl_tvdiff(now, conn->now);
+
+        if(score > highscore) {
+          highscore = score;
+          conn_candidate = conn;
+        }
+      }
+      curr = curr->next;
+    }
+
+    he = Curl_hash_next_element(&iter);
+  }
+
+  return conn_candidate;
+}
 
 /*
  * Given one filled in connection struct (named needle), this function should
@@ -2961,11 +3015,35 @@ ConnectionExists(struct SessionHandle *data,
   return FALSE; /* no matching connecting exists */
 }
 
-/* this connection can now be marked 'idle' */
-static void
-ConnectionDone(struct connectdata *conn)
+/* Mark the connection as 'idle', or close it if the cache is full.
+   Returns TRUE if the connection is kept, or FALSE if it was closed. */
+static bool
+ConnectionDone(struct SessionHandle *data, struct connectdata *conn)
 {
+  /* data->multi->maxconnects can be negative, deal with it. */
+  size_t maxconnects =
+    (data->multi->maxconnects < 0) ? 0 : data->multi->maxconnects;
+  struct connectdata *conn_candidate = NULL;
+
+  /* Mark the current connection as 'unused' */
   conn->inuse = FALSE;
+
+  if(maxconnects > 0 &&
+     data->state.conn_cache->num_connections > maxconnects) {
+    infof(data, "Connection cache is full, closing the oldest one.\n");
+
+    conn_candidate = find_oldest_idle_connection(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);
+    }
+  }
+
+  return (conn_candidate == conn) ? FALSE : TRUE;
 }
 
 /*
@@ -4907,6 +4985,7 @@ static CURLcode create_conn(struct SessionHandle *data,
      * This is a brand new connection, so let's store it in the connection
      * cache of ours!
      */
+    conn->inuse = TRUE;
     ConnectionStore(data, conn);
   }
 
@@ -5182,14 +5261,17 @@ CURLcode Curl_done(struct connectdata **connp,
       result = res2;
   }
   else {
-    ConnectionDone(conn); /* the connection is no longer in use */
-
-    /* remember the most recently used connection */
-    data->state.lastconnect = conn;
-
-    infof(data, "Connection #%ld to host %s left intact\n",
-          conn->connection_id,
-          conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname);
+    /* the connection is no longer in use */
+    if(ConnectionDone(data, conn)) {
+      /* remember the most recently used connection */
+      data->state.lastconnect = conn;
+
+      infof(data, "Connection #%ld to host %s left intact\n",
+            conn->connection_id,
+            conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname);
+    }
+    else
+      data->state.lastconnect = NULL;
   }
 
   *connp = NULL; /* to make the caller of this function better detect that
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index d2a9be1b6..d82534dc8 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -93,7 +93,7 @@ test1379 test1380 test1381 test1382 test1383 test1384 test1385 test1386 \
 test1387 test1388 test1389 test1390 test1391 test1392 test1393 \
 test1400 test1401 test1402 test1403 test1404 test1405 test1406 test1407 \
 test1408 test1409 test1410 test1411 test1412 test1413 \
-test1500 test1501 test1502 test1503 test1504 test1505 \
+test1500 test1501 test1502 test1503 test1504 test1505 test1506 \
 test2000 test2001 test2002 test2003 test2004 test2005 test2006 test2007 \
 test2008 test2009 test2010 test2011 test2012 test2013 test2014 test2015 \
 test2016 test2017 test2018 test2019 test2020 test2021 test2022 \
diff --git a/tests/data/test1506 b/tests/data/test1506
new file mode 100644
index 000000000..8870b2f49
--- /dev/null
+++ b/tests/data/test1506
@@ -0,0 +1,95 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+multi
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data1>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 47
+
+file contents should appear once for each file
+</data1>
+<data2>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 47
+
+file contents should appear once for each file
+</data2>
+<data3>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 47
+
+file contents should appear once for each file
+</data3>
+<data4>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 47
+
+file contents should appear once for each file
+</data4>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+<tool>
+lib1506
+</tool>
+ <name>
+HTTP GET connection cache limit (CURLMOPT_MAXCONNECTS)
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/path/1506 %HOSTIP %HTTPPORT
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+GET /path/15060001 HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Accept: */*

+

+GET /path/15060002 HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Accept: */*

+

+GET /path/15060003 HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Accept: */*

+

+GET /path/15060004 HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Accept: */*

+

+</protocol>
+<strip>
+^Host:.*
+</strip>
+<file name="log/stderr1506" mode="text">
+* Connection #0 to host server1.example.com left intact
+* Connection #1 to host server2.example.com left intact
+* Connection #2 to host server3.example.com left intact
+* Closing connection 0
+* Connection #3 to host server4.example.com left intact
+</file>
+<stripfile>
+$_ = '' if (($_ !~ /left intact/) && ($_ !~ /Closing connection/))
+</stripfile>
+</verify>
+</testcase>
diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc
index d5a36becd..82c265d08 100644
--- a/tests/libtest/Makefile.inc
+++ b/tests/libtest/Makefile.inc
@@ -23,7 +23,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect \
                 lib582 lib583        lib585 lib586 lib587               \
   lib590 lib591                                    lib597 lib598 lib599 \
   \
-  lib1500 lib1501 lib1502 lib1503 lib1504 lib1505
+  lib1500 lib1501 lib1502 lib1503 lib1504 lib1505 lib1506
 
 chkhostname_SOURCES = chkhostname.c ../../lib/curl_gethostname.c
 chkhostname_LDADD = @CURL_NETWORK_LIBS@
@@ -308,3 +308,7 @@ lib1504_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1504
 lib1505_SOURCES = lib1502.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
 lib1505_LDADD = $(TESTUTIL_LIBS)
 lib1505_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1505
+
+lib1506_SOURCES = lib1506.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS)
+lib1506_LDADD = $(TESTUTIL_LIBS)
+lib1506_CPPFLAGS = $(AM_CPPFLAGS) -DLIB1506
diff --git a/tests/libtest/lib1506.c b/tests/libtest/lib1506.c
new file mode 100644
index 000000000..e524beb75
--- /dev/null
+++ b/tests/libtest/lib1506.c
@@ -0,0 +1,132 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) 2013, Linus Nielsen Feltzing <linus@haxx.se>
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://curl.haxx.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ ***************************************************************************/
+#include "test.h"
+
+#include "testutil.h"
+#include "warnless.h"
+#include "memdebug.h"
+
+#define TEST_HANG_TIMEOUT 60 * 1000
+
+#define NUM_HANDLES 4
+
+int test(char *URL)
+{
+  int res = 0;
+  CURL *curl[NUM_HANDLES];
+  int running;
+  CURLM *m = NULL;
+  int i;
+  char target_url[256];
+  char dnsentry[256];
+  struct curl_slist *slist = NULL;
+  char *port = libtest_arg3;
+  char *address = libtest_arg2;
+
+  (void)URL;
+
+  /* Create fake DNS entries for serverX.example.com for all handles */
+  for(i=0; i < NUM_HANDLES; i++) {
+    sprintf(dnsentry, "server%d.example.com:%s:%s", i + 1, port, address);
+    printf("%s\n", dnsentry);
+    slist = curl_slist_append(slist, dnsentry);
+  }
+
+  for(i=0; i < NUM_HANDLES; i++)
+    curl[i] = NULL;
+
+  start_test_timing();
+
+  global_init(CURL_GLOBAL_ALL);
+
+  multi_init(m);
+
+  multi_setopt(m, CURLMOPT_MAXCONNECTS, 3);
+
+  /* get NUM_HANDLES easy handles */
+  for(i=0; i < NUM_HANDLES; i++) {
+    /* get an easy handle */
+    easy_init(curl[i]);
+    /* specify target */
+    sprintf(target_url, "http://server%d.example.com:%s/path/1506%04i",
+            i + 1, port, i + 1);
+    target_url[sizeof(target_url) - 1] = '\0';
+    easy_setopt(curl[i], CURLOPT_URL, target_url);
+    /* go verbose */
+    easy_setopt(curl[i], CURLOPT_VERBOSE, 1L);
+    /* include headers */
+    easy_setopt(curl[i], CURLOPT_HEADER, 1L);
+
+    easy_setopt(curl[i], CURLOPT_RESOLVE, slist);
+  }
+
+  fprintf(stderr, "Start at URL 0\n");
+
+  for(i=0; i < NUM_HANDLES; i++) {
+    /* add handle to multi */
+    multi_add_handle(m, curl[i]);
+
+    for(;;) {
+      struct timeval interval;
+      fd_set rd, wr, exc;
+      int maxfd = -99;
+
+      interval.tv_sec = 1;
+      interval.tv_usec = 0;
+
+      multi_perform(m, &running);
+
+      abort_on_test_timeout();
+
+      if(!running)
+        break; /* done */
+
+      FD_ZERO(&rd);
+      FD_ZERO(&wr);
+      FD_ZERO(&exc);
+
+      multi_fdset(m, &rd, &wr, &exc, &maxfd);
+
+      /* At this point, maxfd is guaranteed to be greater or equal than -1. */
+
+      select_test(maxfd+1, &rd, &wr, &exc, &interval);
+
+      abort_on_test_timeout();
+    }
+  }
+
+test_cleanup:
+
+  /* proper cleanup sequence - type PB */
+
+  for(i=0; i < NUM_HANDLES; i++) {
+    curl_multi_remove_handle(m, curl[i]);
+    curl_easy_cleanup(curl[i]);
+  }
+
+  curl_slist_free_all(slist);
+
+  curl_multi_cleanup(m);
+  curl_global_cleanup();
+
+  return res;
+}