also eliminating use of pipe, thereby addressing #1662

5 files changed, 79 insertions, 108 deletions

diff --git a/ChangeLog b/ChangeLog
index 75185723..fa0da138 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+Fri Mar  4 10:07:18 CET 2011
+        Avoid using a pipe for signalling as well, just use server
+        socket shutdown (also for thread-per-connection). -CG
+
 Thu Mar  3 21:42:47 CET 2011
         Fixing issue where Base64 decode fails when char is defined 
         as unsigned char  (Mantis 1666). -CG/tmayer
diff --git a/src/daemon/connection.c b/src/daemon/connection.c
index 49fa6ffe..c7e42af6 100644
--- a/src/daemon/connection.c
+++ b/src/daemon/connection.c
@@ -771,6 +771,7 @@ MHD_connection_get_fdset (struct MHD_Connection *connection,
   return ret;
 }
 
+
 /**
  * Obtain the pollfd for this connection
  *
diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
index 460e057e..f12f8207 100644
--- a/src/daemon/daemon.c
+++ b/src/daemon/daemon.c
@@ -547,6 +547,7 @@ MHD_get_fdset (struct MHD_Daemon *daemon,
   return MHD_YES;
 }
 
+
 /**
  * Main function of the thread that handles an individual
  * connection when MHD_USE_THREAD_PER_CONNECTION is set.
@@ -563,6 +564,7 @@ MHD_handle_connection (void *data)
   fd_set ws;
   fd_set es;
   int max;
+  int fd;
   struct timeval tv;
   struct timeval *tvp;
   unsigned int timeout;
@@ -602,24 +604,27 @@ MHD_handle_connection (void *data)
           FD_ZERO (&rs);
           FD_ZERO (&ws);
           FD_ZERO (&es);
-          if (-1 != con->daemon->wpipe[0])
+          if (-1 != (fd = con->daemon->socket_fd))
             {
-              max = con->daemon->wpipe[0];
-              FD_SET (con->daemon->wpipe[0], &rs);
+              /* we add the listen socket to our read set so
+                 that if the server is shutdown, we notice it */
+              max = fd;
+              FD_SET (fd, &rs);
             }
           else
             max = 0;
           MHD_connection_get_fdset (con, &rs, &ws, &es, &max);
           num_ready = SELECT (max + 1, &rs, &ws, &es, tvp);
-          if (num_ready < 0) {
-            if (errno == EINTR)
-              continue;
+          if (num_ready < 0) 
+            {
+              if (errno == EINTR)
+                continue;
 #if HAVE_MESSAGES
-            MHD_DLOG (con->daemon, "Error during select (%d): `%s'\n", max,
-                      STRERROR (errno));
+              MHD_DLOG (con->daemon, "Error during select (%d): `%s'\n", max,
+                        STRERROR (errno));
 #endif
-            break;
-          }
+              break;
+            }
           /* call appropriate connection handler if necessary */
           if ((con->socket_fd != -1) && (FD_ISSET (con->socket_fd, &rs)))
             con->read_handler (con);
@@ -640,10 +645,20 @@ MHD_handle_connection (void *data)
             p[0].events |= POLLIN;        
           if (mp.events & MHD_POLL_ACTION_OUT) 
             p[0].events |= POLLOUT;
-          p[1].fd = con->daemon->wpipe[0];
-          p[1].events |= POLLIN;        
+          /* we add the listen socket to our read set so
+             that if the server is shutdown, we notice it */
+
+          fd = con->daemon->socket_fd;
+          p[1].fd = fd;
+          p[1].events |= POLLIN | POLLRDHUP;
           
-          if (poll (p, 2, (tvp == NULL) ? -1 : (tv.tv_sec * 1000)) < 0) 
+          if (poll (p, 
+                    (fd != -1) ? 2 : 1, 
+                    (tvp == NULL) 
+                    ? -1 
+                    : ((fd != -1) 
+                       ? (tv.tv_sec * 1000) 
+                       : 0)) < 0) 
             {
               if (errno == EINTR)
                 continue;
@@ -689,7 +704,9 @@ MHD_handle_connection (void *data)
  * @return number of bytes actually received
  */
 static ssize_t
-recv_param_adapter (struct MHD_Connection *connection, void *other, size_t i)
+recv_param_adapter (struct MHD_Connection *connection,
+                    void *other, 
+                    size_t i)
 {
   if (connection->socket_fd == -1)
     return -1;
@@ -708,7 +725,8 @@ recv_param_adapter (struct MHD_Connection *connection, void *other, size_t i)
  */
 static ssize_t
 send_param_adapter (struct MHD_Connection *connection,
-                    const void *other, size_t i)
+                    const void *other,
+                    size_t i)
 {
 #if LINUX
   int fd;
@@ -841,7 +859,6 @@ create_thread (pthread_t * thread,
 }
 
 
-
 /**
  * Accept an incoming connection and create the MHD_Connection object for
  * it.  This function also enforces policy by way of checking with the
@@ -1136,7 +1153,8 @@ MHD_cleanup_connections (struct MHD_Daemon *daemon)
  *        necessiate the use of a timeout right now).
  */
 int
-MHD_get_timeout (struct MHD_Daemon *daemon, unsigned MHD_LONG_LONG *timeout)
+MHD_get_timeout (struct MHD_Daemon *daemon,
+                 unsigned MHD_LONG_LONG *timeout)
 {
   time_t earliest_deadline;
   time_t now;
@@ -1179,7 +1197,8 @@ MHD_get_timeout (struct MHD_Daemon *daemon, unsigned MHD_LONG_LONG *timeout)
  * @return MHD_NO on serious errors, MHD_YES on success
  */
 static int
-MHD_select (struct MHD_Daemon *daemon, int may_block)
+MHD_select (struct MHD_Daemon *daemon, 
+            int may_block)
 {
   struct MHD_Connection *pos;
   int num_ready;
@@ -1199,13 +1218,7 @@ MHD_select (struct MHD_Daemon *daemon, int may_block)
   FD_ZERO (&rs);
   FD_ZERO (&ws);
   FD_ZERO (&es);
-  if (-1 != daemon->wpipe[0])
-    {
-      max = daemon->wpipe[0];
-      FD_SET (daemon->wpipe[0], &rs);
-    }
-  else
-    max = 0;
+  max = 0;
   if (0 == (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
     {
       /* single-threaded, go over everything */
@@ -1307,18 +1320,16 @@ MHD_poll (struct MHD_Daemon *daemon,
       pos = pos->next;
     }
   {
-    struct pollfd p[2 + num_connections];
+    struct pollfd p[1 + num_connections];
     struct MHD_Pollfd mp;
     unsigned MHD_LONG_LONG ltimeout;
     unsigned int i;
     int timeout;
     
+    memset (p, 0, sizeof (p));
     p[0].fd = daemon->socket_fd;
-    p[0].events = POLLIN;
+    p[0].events = POLLIN | POLLRDHUP;
     p[0].revents = 0;
-    p[1].fd = daemon->wpipe[0];
-    p[1].events = POLLIN;
-    p[1].revents = 0;
     if (may_block == MHD_NO)
       timeout = 0;
     else if (MHD_YES != MHD_get_timeout (daemon, &ltimeout))
@@ -1332,16 +1343,15 @@ MHD_poll (struct MHD_Daemon *daemon,
       {
         memset(&mp, 0, sizeof (struct MHD_Pollfd));
         MHD_connection_get_pollfd (pos, &mp);
-        memset(&p, 0, sizeof (p));
-        p[2+i].fd = mp.fd;
+        p[1+i].fd = mp.fd;
         if (mp.events & MHD_POLL_ACTION_IN) 
-          p[2+i].events |= POLLIN;        
+          p[1+i].events |= POLLIN;        
         if (mp.events & MHD_POLL_ACTION_OUT) 
-          p[2+i].events |= POLLOUT;
+          p[1+i].events |= POLLOUT;
         i++;
         pos = pos->next;
       }
-    if (poll (p, 2 + num_connections, timeout) < 0) {
+    if (poll (p, 1 + num_connections, timeout) < 0) {
       if (errno == EINTR)
         return MHD_YES;
 #if HAVE_MESSAGES
@@ -1354,21 +1364,29 @@ MHD_poll (struct MHD_Daemon *daemon,
       return MHD_NO;  
     if (daemon->socket_fd < 0) 
       return MHD_YES; 
-    if (0 != (p[0].revents & POLLIN)) 
-      MHD_accept_connection (daemon);
     i = 0;
     pos = daemon->connections;
     while (pos != NULL)
       {
-        if (0 != (p[2+i].revents & POLLIN)) 
+        /* first, sanity checks */
+        if (i >= num_connections)
+          break; /* connection list changed somehow, retry later ... */
+        MHD_connection_get_pollfd (pos, &mp);
+        if (p[1+i].fd != mp.fd)
+          break; /* fd mismatch, something else happened, retry later ... */
+
+        /* normal handling */
+        if (0 != (p[1+i].revents & POLLIN)) 
           pos->read_handler (pos);
-        if (0 != (p[2+i].revents & POLLOUT)) 
+        if (0 != (p[1+i].revents & POLLOUT)) 
           pos->write_handler (pos);
         if (pos->socket_fd != -1)
           pos->idle_handler (pos);
         i++;
         pos = pos->next;
       }
+    if (0 != (p[0].revents & POLLIN)) 
+      MHD_accept_connection (daemon);
   }
   return MHD_YES;
 #else
@@ -1797,37 +1815,6 @@ MHD_start_daemon_va (unsigned int options,
   retVal->pool_size = MHD_POOL_SIZE_DEFAULT;
   retVal->unescape_callback = &MHD_http_unescape;
   retVal->connection_timeout = 0;       /* no timeout */
-  retVal->wpipe[0] = -1;
-  retVal->wpipe[1] = -1;
-  if ( (0 != (options & MHD_USE_THREAD_PER_CONNECTION)) ||
-       (0 != (options & MHD_USE_SELECT_INTERNALLY)) )
-    {
-      if (0 != pipe (retVal->wpipe))
-        {
-#if HAVE_MESSAGES
-          FPRINTF(stderr, 
-                  "Failed to create control pipe: %s\n",
-                  STRERROR (errno));
-#endif
-          free (retVal);
-          return NULL;
-        }
-#ifndef WINDOWS
-      if ( (0 == (options & MHD_USE_POLL)) &&
-           (retVal->wpipe[0] >= FD_SETSIZE) )
-        {
-#if HAVE_MESSAGES
-          FPRINTF(stderr, 
-                  "file descriptor for control pipe exceeds maximum value\n");
-#endif
-          close (retVal->wpipe[0]);
-          close (retVal->wpipe[1]);
-          free (retVal);
-          return NULL;
-        }
-#endif
-    }
-
 #ifdef DAUTH_SUPPORT
   retVal->digest_auth_rand_size = 0;
   retVal->digest_auth_random = NULL;
@@ -2217,6 +2204,7 @@ thread_failed:
   return NULL;
 }
 
+
 /**
  * Close all connections for the daemon
  */
@@ -2239,6 +2227,7 @@ MHD_close_connections (struct MHD_Daemon *daemon)
     }
 }
 
+
 /**
  * Shutdown an http daemon
  */
@@ -2249,15 +2238,12 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
   int fd;
   unsigned int i;
   int rc;
-  char c;
 
   if (daemon == NULL)
     return;
   daemon->shutdown = MHD_YES;
   fd = daemon->socket_fd;
   daemon->socket_fd = -1;
-  if (daemon->wpipe[1] != -1)
-    write (daemon->wpipe[1], "e", 1);
 
   /* Prepare workers for shutdown */
   for (i = 0; i < daemon->worker_pool_size; ++i)
@@ -2265,20 +2251,13 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
       daemon->worker_pool[i].shutdown = MHD_YES;
       daemon->worker_pool[i].socket_fd = -1;
     }
-
-#if OSX
-  /* without this, either (thread pool = 0) threads would get stuck or
-   * CLOSE would get stuck if attempted before (thread pool > 0)
-   * threads have ended */
   SHUTDOWN (fd, SHUT_RDWR);
-#else
 #if DEBUG_CLOSE
 #if HAVE_MESSAGES
-  MHD_DLOG (daemon, "MHD shutdown, closing listen socket\n");
+  MHD_DLOG (daemon, "MHD listen socket shutdown\n");
 #endif
 #endif
-  CLOSE (fd);
-#endif
+
 
   /* Signal workers to stop and clean them up */
   for (i = 0; i < daemon->worker_pool_size; ++i)
@@ -2309,15 +2288,7 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
         }
     }
   MHD_close_connections (daemon);
-
-#if OSX
-#if DEBUG_CLOSE
-#if HAVE_MESSAGES
-  MHD_DLOG (daemon, "MHD shutdown, closing listen socket\n");
-#endif
-#endif
   CLOSE (fd);
-#endif
 
   /* TLS clean up */
 #if HTTPS_SUPPORT
@@ -2350,17 +2321,10 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
 #endif
   pthread_mutex_destroy (&daemon->per_ip_connection_mutex);
 
-  if (daemon->wpipe[1] != -1)
-    {
-      /* just to be sure, remove the one char we 
-         wrote into the pipe */
-      (void) read (daemon->wpipe[0], &c, 1);
-      close (daemon->wpipe[0]);
-      close (daemon->wpipe[1]);
-    }
   free (daemon);
 }
 
+
 /**
  * Obtain information about the given daemon
  * (not fully implemented!).
@@ -2384,6 +2348,7 @@ MHD_get_daemon_info (struct MHD_Daemon *daemon,
     };
 }
 
+
 /**
  * Sets the global error handler to a different implementation.  "cb"
  * will only be called in the case of typically fatal, serious
@@ -2405,6 +2370,7 @@ void MHD_set_panic_func (MHD_PanicCallback cb, void *cls)
   mhd_panic_cls = cls;
 }
 
+
 /**
  * Obtain the version of this library
  *
@@ -2416,6 +2382,7 @@ MHD_get_version (void)
   return PACKAGE_VERSION;
 }
 
+
 #ifdef __GNUC__
 #define ATTRIBUTE_CONSTRUCTOR __attribute__ ((constructor))
 #define ATTRIBUTE_DESTRUCTOR __attribute__ ((destructor))
@@ -2428,10 +2395,12 @@ MHD_get_version (void)
 GCRY_THREAD_OPTION_PTHREAD_IMPL;
 #endif
 
+
 /**
  * Initialize do setup work.
  */
-void ATTRIBUTE_CONSTRUCTOR MHD_init ()
+void ATTRIBUTE_CONSTRUCTOR 
+MHD_init ()
 {
   mhd_panic = &mhd_panic_std;
   mhd_panic_cls = NULL;
@@ -2447,7 +2416,9 @@ void ATTRIBUTE_CONSTRUCTOR MHD_init ()
 #endif
 }
 
-void ATTRIBUTE_DESTRUCTOR MHD_fini ()
+
+void ATTRIBUTE_DESTRUCTOR 
+MHD_fini ()
 {
 #if HTTPS_SUPPORT
   gnutls_global_deinit ();
diff --git a/src/daemon/internal.h b/src/daemon/internal.h
index ee499cef..9a3eb8cb 100644
--- a/src/daemon/internal.h
+++ b/src/daemon/internal.h
@@ -857,11 +857,6 @@ struct MHD_Daemon
   int socket_fd;
 
   /**
-   * Pipe we use to signal shutdown.
-   */
-  int wpipe[2];
-
-  /**
    * Are we shutting down?
    */
   int shutdown;
diff --git a/src/examples/minimal_example.c b/src/examples/minimal_example.c
index 8c7d17d9..5bf63444 100644
--- a/src/examples/minimal_example.c
+++ b/src/examples/minimal_example.c
@@ -67,13 +67,13 @@ main (int argc, char *const *argv)
       printf ("%s PORT\n", argv[0]);
       return 1;
     }
-  d = MHD_start_daemon (MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG | MHD_USE_POLL,
-                        // MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG,
+  d = MHD_start_daemon (// MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG | MHD_USE_POLL,
+                        MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG,
                         // MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG | MHD_USE_POLL,
                         // MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG,
                         atoi (argv[1]),
                         NULL, NULL, &ahc_echo, PAGE,
-                        MHD_OPTION_CONNECTION_TIMEOUT, (unsigned int) 5,
+                        MHD_OPTION_CONNECTION_TIMEOUT, (unsigned int) 120,
                         MHD_OPTION_END);
   if (d == NULL)
     return 1;