fix thread-pool connection-limit shutdown issue, adding testcase

5 files changed, 280 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index 8f81473f..e4807d32 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+Sat Apr  4 17:48:13 CEST 2015
+        Fix issue in thread-pool mode where a MHD_stop_daemon()
+        might not reach threads that stopped listening because
+        we hit the maximum number of concurrent connections and
+        the option MHD_USE_PIPE_FOR_SHUTDOWN was also not used.
+        Testcase added as well. -CG
+
 Fri Apr  3 12:55:31 CEST 2015
         Update HTTPS testcases to avoid SSLv3, as SSLv3 is dead.
 
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index 7f4449e5..bdf08e00 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -2224,9 +2224,13 @@ MHD_select (struct MHD_Daemon *daemon,
         return MHD_NO;
 
       /* If we're at the connection limit, no need to
-         accept new connections. */
-      if ( (daemon->connections == daemon->connection_limit) &&
-           (MHD_INVALID_SOCKET != daemon->socket_fd) )
+         accept new connections; however, make sure
+         we do not miss the shutdown, so only do this
+         optimization if we have a shutdown signaling
+         pipe. */
+      if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
+           (daemon->connections == daemon->connection_limit) &&
+           (0 != (daemon->options & MHD_USE_PIPE_FOR_SHUTDOWN)) )
         FD_CLR (daemon->socket_fd, &rs);
     }
   else
@@ -2672,7 +2676,7 @@ MHD_epoll (struct MHD_Daemon *daemon,
               while ( (MHD_YES == MHD_accept_connection (daemon)) &&
                       (daemon->connections < daemon->connection_limit) &&
                       (series_length < 128) )
-                      series_length++;
+                series_length++;
             }
         }
     }
diff --git a/src/testcurl/Makefile.am b/src/testcurl/Makefile.am
index d418206d..2bedaaca 100644
--- a/src/testcurl/Makefile.am
+++ b/src/testcurl/Makefile.am
@@ -30,6 +30,7 @@ check_PROGRAMS = \
   test_get_sendfile \
   test_urlparse \
   test_put \
+  test_concurrent_stop \
   test_process_headers \
   test_process_arguments \
   test_parse_cookies \
@@ -85,6 +86,12 @@ test_start_stop_SOURCES = \
 test_start_stop_LDADD = \
   $(top_builddir)/src/microhttpd/libmicrohttpd.la
 
+test_concurrent_stop_SOURCES = \
+  test_concurrent_stop.c
+test_concurrent_stop_LDADD = \
+  $(top_builddir)/src/microhttpd/libmicrohttpd.la \
+ @LIBCURL@
+
 test_options_SOURCES = \
   test_options.c
 test_options_LDADD = \
@@ -292,4 +299,3 @@ test_timeout_SOURCES = \
 test_timeout_LDADD = \
   $(top_builddir)/src/microhttpd/libmicrohttpd.la \
   @LIBCURL@
-
diff --git a/src/testcurl/test_concurrent_stop.c b/src/testcurl/test_concurrent_stop.c
new file mode 100644
index 00000000..b9dbc03b
--- /dev/null
+++ b/src/testcurl/test_concurrent_stop.c
@@ -0,0 +1,228 @@
+/*
+     This file is part of libmicrohttpd
+     Copyright (C) 2007, 2009, 2011, 2015 Christian Grothoff
+
+     libmicrohttpd is free software; you can redistribute it and/or modify
+     it under the terms of the GNU General Public License as published
+     by the Free Software Foundation; either version 3, or (at your
+     option) any later version.
+
+     libmicrohttpd is distributed in the hope that it will be useful, but
+     WITHOUT ANY WARRANTY; without even the implied warranty of
+     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+     General Public License for more details.
+
+     You should have received a copy of the GNU General Public License
+     along with libmicrohttpd; see the file COPYING.  If not, write to the
+     Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+     Boston, MA 02111-1307, USA.
+*/
+
+/**
+ * @file test_concurrent_stop.c
+ * @brief test stopping server while concurrent GETs are ongoing
+ * @author Christian Grothoff
+ */
+#include "MHD_config.h"
+#include "platform.h"
+#include <curl/curl.h>
+#include <microhttpd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
+#include "gauger.h"
+
+#ifdef CPU_COUNT
+#undef CPU_COUNT
+#endif
+#define CPU_COUNT 40
+
+
+/**
+ * How many rounds of operations do we do for each
+ * test (total number of requests will be ROUNDS * PAR).
+ */
+#define ROUNDS 50000
+
+/**
+ * How many requests do we do in parallel?
+ */
+#define PAR CPU_COUNT
+
+/**
+ * Do we use HTTP 1.1?
+ */
+static int oneone;
+
+/**
+ * Response to return (re-used).
+ */
+static struct MHD_Response *response;
+
+
+static size_t
+copyBuffer (void *ptr,
+            size_t size, size_t nmemb,
+            void *ctx)
+{
+  return size * nmemb;
+}
+
+static int
+ahc_echo (void *cls,
+          struct MHD_Connection *connection,
+          const char *url,
+          const char *method,
+          const char *version,
+          const char *upload_data, size_t *upload_data_size,
+          void **unused)
+{
+  static int ptr;
+  const char *me = cls;
+  int ret;
+
+  if (0 != strcmp (me, method))
+    return MHD_NO;              /* unexpected method */
+  if (&ptr != *unused)
+    {
+      *unused = &ptr;
+      return MHD_YES;
+    }
+  *unused = NULL;
+  ret = MHD_queue_response (connection,
+                            MHD_HTTP_OK,
+                            response);
+  if (ret == MHD_NO)
+    abort ();
+  return ret;
+}
+
+
+static pid_t
+do_gets (int port)
+{
+  pid_t ret;
+  CURL *c;
+  CURLcode errornum;
+  unsigned int i;
+  unsigned int j;
+  pid_t par[PAR];
+  char url[64];
+
+  sprintf(url, "http://127.0.0.1:%d/hello_world", port);
+
+  ret = fork ();
+  if (ret == -1) abort ();
+  if (ret != 0)
+    return ret;
+  for (j=0;j<PAR;j++)
+    {
+      par[j] = fork ();
+      if (par[j] == 0)
+        {
+          for (i=0;i<ROUNDS;i++)
+            {
+              c = curl_easy_init ();
+              curl_easy_setopt (c, CURLOPT_URL, url);
+              curl_easy_setopt (c, CURLOPT_WRITEFUNCTION, &copyBuffer);
+              curl_easy_setopt (c, CURLOPT_WRITEDATA, NULL);
+              curl_easy_setopt (c, CURLOPT_FAILONERROR, 1);
+              curl_easy_setopt (c, CURLOPT_TIMEOUT, 150L);
+              if (oneone)
+                curl_easy_setopt (c, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1);
+              else
+                curl_easy_setopt (c, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_0);
+              curl_easy_setopt (c, CURLOPT_CONNECTTIMEOUT, 150L);
+              /* NOTE: use of CONNECTTIMEOUT without also
+                 setting NOSIGNAL results in really weird
+                 crashes on my system! */
+              curl_easy_setopt (c, CURLOPT_NOSIGNAL, 1);
+              if (CURLE_OK != (errornum = curl_easy_perform (c)))
+                {
+                  curl_easy_cleanup (c);
+                  _exit (1);
+                }
+              curl_easy_cleanup (c);
+            }
+          _exit (0);
+        }
+    }
+  for (j=0;j<PAR;j++)
+    waitpid (par[j], NULL, 0);
+  _exit (0);
+}
+
+
+static void
+join_gets (pid_t pid)
+{
+  int status;
+
+  status = 1;
+  waitpid (pid, &status, 0);
+  if (0 != status)
+    abort ();
+}
+
+
+static int
+testMultithreadedGet (int port, int poll_flag)
+{
+  struct MHD_Daemon *d;
+  pid_t p;
+
+  d = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG  | poll_flag,
+                        port, NULL, NULL, &ahc_echo, "GET", MHD_OPTION_END);
+  if (d == NULL)
+    return 16;
+  p = do_gets (port);
+  sleep (1);
+  MHD_stop_daemon (d);
+  join_gets (p);
+  return 0;
+}
+
+
+static int
+testMultithreadedPoolGet (int port, int poll_flag)
+{
+  struct MHD_Daemon *d;
+  pid_t p;
+
+  d = MHD_start_daemon (MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG | poll_flag,
+                        port,
+                        NULL, NULL,
+                        &ahc_echo, "GET",
+                        MHD_OPTION_THREAD_POOL_SIZE, CPU_COUNT,
+                        MHD_OPTION_END);
+  if (d == NULL)
+    return 16;
+  p = do_gets (port);
+  sleep (1);
+  MHD_stop_daemon (d);
+  join_gets (p);
+  return 0;
+}
+
+
+
+int
+main (int argc, char *const *argv)
+{
+  unsigned int errorCount = 0;
+  int port = 1081;
+
+  oneone = NULL != strstr (argv[0], "11");
+  if (0 != curl_global_init (CURL_GLOBAL_WIN32))
+    return 2;
+  response = MHD_create_response_from_buffer (strlen ("/hello_world"),
+                                              "/hello_world",
+                                              MHD_RESPMEM_MUST_COPY);
+  // errorCount += testMultithreadedGet (port++, 0);
+  errorCount += testMultithreadedPoolGet (port++, 0);
+  MHD_destroy_response (response);
+  if (errorCount != 0)
+    fprintf (stderr, "Error (code: %u)\n", errorCount);
+  curl_global_cleanup ();
+  return errorCount != 0;       /* 0 == pass */
+}
diff --git a/src/testcurl/test_iplimit.c b/src/testcurl/test_iplimit.c
index 05d61c08..f8923719 100644
--- a/src/testcurl/test_iplimit.c
+++ b/src/testcurl/test_iplimit.c
@@ -110,13 +110,16 @@ testMultithreadedGet ()
   struct MHD_Daemon *d;
   char buf[2048];
   int k;
+  unsigned int success;
+  unsigned int failure;
 
   /* Test only valid for HTTP/1.1 (uses persistent connections) */
   if (!oneone)
     return 0;
 
   d = MHD_start_daemon (MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG,
-                        1081, NULL, NULL, &ahc_echo, "GET",
+                        1081, NULL, NULL,
+                        &ahc_echo, "GET",
                         MHD_OPTION_PER_IP_CONNECTION_LIMIT, 2,
                         MHD_OPTION_END);
   if (d == NULL)
@@ -128,11 +131,16 @@ testMultithreadedGet ()
       CURL *cenv[3];
       int i;
 
+      fprintf (stderr,
+               "Iteration %d\n",
+               k);
+      success = 0;
+      failure = 0;
       for (i = 0; i < 3; ++i)
         {
           CURL *c;
           CURLcode errornum;
- 
+
           cenv[i] = c = curl_easy_init ();
           cbc[i].buf = buf;
           cbc[i].size = 2048;
@@ -152,32 +160,25 @@ testMultithreadedGet ()
           curl_easy_setopt (c, CURLOPT_NOSIGNAL, 1);
 
           errornum = curl_easy_perform (c);
-          if ( ( (CURLE_OK != errornum) && (i <  2) ) ||
-               ( (CURLE_OK == errornum) && (i == 2) ) )
-            {
-              int j;
-
-              /* First 2 should succeed */
-              if (i < 2)
-                fprintf (stderr,
-                         "curl_easy_perform failed: `%s'\n",
-                         curl_easy_strerror (errornum));
-
-              /* Last request should have failed */
-              else
-                fprintf (stderr,
-                         "No error on IP address over limit\n");
-
-              for (j = 0; j < i; ++j)
-                curl_easy_cleanup (cenv[j]);
-              MHD_stop_daemon (d);
-              return 32;
-            }
+          if (CURLE_OK == errornum)
+            success++;
+          else
+            failure++;
         }
 
       /* Cleanup the environments */
       for (i = 0; i < 3; ++i)
         curl_easy_cleanup (cenv[i]);
+      if ( (2 != success) ||
+           (1 != failure) )
+      {
+        fprintf (stderr,
+                 "Unexpected number of success (%u) or failure (%u)\n",
+                 success,
+                 failure);
+        MHD_stop_daemon (d);
+        return 32;
+      }
 
       sleep(2);
 
@@ -194,8 +195,6 @@ testMultithreadedGet ()
               return 128;
             }
         }
-
-
     }
   MHD_stop_daemon (d);
   return 0;
@@ -226,11 +225,14 @@ testMultithreadedPoolGet ()
       CURL *cenv[3];
       int i;
 
+      fprintf (stderr,
+               "Iteration %d\n",
+               k);
       for (i = 0; i < 3; ++i)
         {
           CURL *c;
           CURLcode errornum;
- 
+
           cenv[i] = c = curl_easy_init ();
           cbc[i].buf = buf;
           cbc[i].size = 2048;
@@ -307,8 +309,8 @@ main (int argc, char *const *argv)
   oneone = NULL != strstr (argv[0], "11");
   if (0 != curl_global_init (CURL_GLOBAL_WIN32))
     return 2;
-  errorCount += testMultithreadedGet ();
-  errorCount += testMultithreadedPoolGet ();
+  errorCount |= testMultithreadedGet ();
+  errorCount |= testMultithreadedPoolGet ();
   if (errorCount != 0)
     fprintf (stderr, "Error (code: %u)\n", errorCount);
   curl_global_cleanup ();