[GNUnet-SVN] r16980 - in libmicrohttpd: . src/daemon

[gnunet]

Author: grothoff
Date: 2011-09-21 09:47:03 +0200 (Wed, 21 Sep 2011)
New Revision: 16980

Modified:
   libmicrohttpd/AUTHORS
   libmicrohttpd/src/daemon/daemon.c
   libmicrohttpd/src/daemon/internal.h
Log:
From: 
Will Bryant <address@hidden>
  To: 
libmicrohttpd development and user mailinglist <address@hidden>
  Date: 
Yesterday 03:59:23 PM
  Attachments: 
 0001-use-a-pipe-to-signal-shutdown-to-select-and-poll.patch
   
I attach my current work.  I have attempted to preserve and restore as much of 
the 'old' pipe shutdown code that was present in earlier versions of the 
library, but have removed the sections that aren't relevant now that there is a 
socket-by-socket client shutdown procedure.

On OS X, all the 'regular' tests pass.  The two perf test programs both fail on 
a random subset of the tests with "Failed to bind to port 1081: Address already 
in use" (or the other ports that the perf tests use).

I initially thought that this was a problem with this patch, but after I 
couldn't find any problem with it I tried backporting the perf test programs to 
the older 0.9.7 version of libmicrohttpd, which is before the listening socket 
shutdown code went in, and it turned out that the older versions failed in the 
same way.

On OpenIndiana (oi_151a - but should behave the same as other 
SunOS/Solaris/OpenSolaris/Illumos kernels), the same problem occurs.  (I 
haven't tried the backport there too.)

Linux is known to have slightly different behavior for the SOL_REUSEADDR option 
which might explain this, but it's quite possible that this is just a timing 
issue with the cleanup of the client threads in these perf test programs (if 
you wait for 1s between the tests then the problem goes away).  

On OS X, using both debugging printfs and shelling out to lsof -itcp, I have 
confirmed that the listening socket has been closed successfully before the 
error occurs.   So I am starting to suspect that it is not possible to bind to 
an address that was used as a server socket until all the client sockets have 
also been cleaned up - but that's just a theory at this point.

Anyway, as far as this patch goes, OS X looks a lot healthier - as healthy as 
it was in 0.9.7 (0.9.8 and 0.9.9 both failed to build entirely).

There is one further test failure on openindiana, which I believe is not 
related to this patch - all the same 'regular' tests pass except 
daemontest_get_response_cleanup, which dies due to an unhandled SIGPIPE when 
trying to write to the socket after killing the curl process:

Program received signal SIGPIPE, Broken pipe.
[Switching to Thread 2 (LWP 2)]
0xfed634f7 in __so_send () from /lib/libc.so.1
(gdb) bt
#0  0xfed634f7 in __so_send () from /lib/libc.so.1
#1  0xfed511d7 in _so_send () from /lib/libc.so.1
#2  0xfe7dcb26 in send () from /lib/libsocket.so.1
#3  0xfef56d29 in send_param_adapter (connection=0x20, other=0xfe99080f, i=0) 
at daemon.c:764
#4  0xfef54a0b in do_write (connection=0x8073ad0) at connection.c:1495
#5  0xfef54f6f in MHD_connection_handle_write (connection=0x8073ad0) at 
connection.c:1909
#6  0xfef57b3c in MHD_select (daemon=0x80737a8, may_block=0) at daemon.c:1367
#7  0xfef57ff4 in MHD_select_thread (cls=0x80737a8) at daemon.c:1615
#8  0xfed5f0f3 in _thrp_setup () from /lib/libc.so.1
#9  0xfed5f3a0 in ?? () from /lib/libc.so.1
#10 0xfe9a0240 in ?? ()
#11 0x00000000 in ?? ()
(gdb) info threads
* 4 Thread 2 (LWP 2)  0xfed634f7 in __so_send () from /lib/libc.so.1
  3 LWP    2          0xfed634f7 in __so_send () from /lib/libc.so.1
  2 Thread 1 (LWP 1)  0xfed63e87 in __waitid () from /lib/libc.so.1
  1 LWP    1          0xfed63e87 in __waitid () from /lib/libc.so.1
(gdb) thread 2
[Switching to thread 2 (Thread 1 (LWP 1))]#0  0xfed63e87 in __waitid () from 
/lib/libc.so.1
(gdb) bt
#0  0xfed63e87 in __waitid () from /lib/libc.so.1
#1  0xfed515bd in waitid () from /lib/libc.so.1
#2  0xfed00035 in waitpid () from /lib/libc.so.1
#3  0x08051423 in kill_curl (pid=4479) at daemontest_get_response_cleanup.c:70
#4  0x0805157d in main (argc=1, argv=0x6b) at 
daemontest_get_response_cleanup.c:141

0001-use-a-pipe-to-signal-shutdown-to-select-and-poll.patch


Modified: libmicrohttpd/AUTHORS
===================================================================
--- libmicrohttpd/AUTHORS       2011-09-21 07:44:10 UTC (rev 16979)
+++ libmicrohttpd/AUTHORS       2011-09-21 07:47:03 UTC (rev 16980)
@@ -33,6 +33,7 @@
 Thomas Stalder <address@hidden>
 Zhimin Huang <address@hidden>
 Jan Seeger <address@hidden>
+Will Bryant <address@hidden>
 
 Documentation contributions also came from:
 Marco Maggi <address@hidden>

Modified: libmicrohttpd/src/daemon/daemon.c
===================================================================
--- libmicrohttpd/src/daemon/daemon.c   2011-09-21 07:44:10 UTC (rev 16979)
+++ libmicrohttpd/src/daemon/daemon.c   2011-09-21 07:47:03 UTC (rev 16980)
@@ -1307,6 +1307,15 @@
       FD_SET (max, &rs);
     }
 
+#ifndef HAVE_LISTEN_SHUTDOWN
+  if (-1 == daemon->wpipe[0])
+    return MHD_NO;
+  FD_SET (daemon->wpipe[0], &rs);
+  /* update max file descriptor */
+  if (max < daemon->wpipe[0])
+    max = daemon->wpipe[0];
+#endif
+
   tv = NULL;
   if (may_block == MHD_NO)
     {
@@ -1389,7 +1398,11 @@
       pos = pos->next;
     }
   {
+ #ifdef HAVE_LISTEN_SHUTDOWN
     struct pollfd p[1 + num_connections];
+ #else
+    struct pollfd p[2 + num_connections];
+ #endif
     struct MHD_Pollfd mp;
     unsigned MHD_LONG_LONG ltimeout;
     unsigned int i;
@@ -1399,10 +1412,17 @@
     memset (p, 0, sizeof (p));
     if ( (daemon->max_connections > 0) && (daemon->socket_fd != -1) )
       {
-       poll_server = 1;
        p[0].fd = daemon->socket_fd;
        p[0].events = POLLIN;
        p[0].revents = 0;
+#ifdef HAVE_LISTEN_SHUTDOWN
+       poll_server = 1;
+#else
+       p[1].fd = daemon->wpipe[0];
+       p[1].events = POLLIN;
+       p[1].revents = 0;
+       poll_server = 2;
+#endif
       }
     else
       {
@@ -1464,7 +1484,7 @@
        pos->idle_handler (pos);
        i++;
       }
-    if ( (1 == poll_server) &&
+    if ( (0 != poll_server) &&
         (0 != (p[0].revents & POLLIN)) )
       MHD_accept_connection (daemon);
   }
@@ -1483,22 +1503,27 @@
 MHD_poll_listen_socket (struct MHD_Daemon *daemon,
                        int may_block)
 {
-  struct pollfd p;
+#ifdef HAVE_LISTEN_SHUTDOWN
+  struct pollfd p[1];
+#else
+  struct pollfd p[2];
+#endif
   int timeout;
   
   memset (&p, 0, sizeof (p));
-  p.fd = daemon->socket_fd;
-  p.events = POLLIN;
-  p.revents = 0;
+  p[0].fd = daemon->socket_fd;
+  p[0].events = POLLIN;
+  p[0].revents = 0;
+#ifndef HAVE_LISTEN_SHUTDOWN
+  p[1].fd = daemon->wpipe[0];
+  p[1].events = POLLIN;
+  p[1].revents = 0;
+#endif
   if (may_block == MHD_NO)
     timeout = 0;
   else
     timeout = -1;
-#ifdef __CYGWIN__
-  /* See https://gnunet.org/bugs/view.php?id=1674 */
-  timeout = ( (timeout > 2000) || (timeout < 0) ) ? 2000 : timeout;
-#endif
-  if (poll (&p, 1, timeout) < 0)
+  if (poll (p, (sizeof(p)/sizeof(struct pollfd)), timeout) < 0)
     {
       if (errno == EINTR)
        return MHD_YES;
@@ -1512,7 +1537,7 @@
     return MHD_NO;  
   if (daemon->socket_fd < 0) 
     return MHD_YES; 
-  if (0 != (p.revents & POLLIN))
+  if (0 != (p[0].revents & POLLIN))
     MHD_accept_connection (daemon);  
   return MHD_YES;
 }
@@ -1971,6 +1996,34 @@
   retVal->pool_size = MHD_POOL_SIZE_DEFAULT;
   retVal->unescape_callback = &MHD_http_unescape;
   retVal->connection_timeout = 0;       /* no timeout */
+#ifndef HAVE_LISTEN_SHUTDOWN
+  retVal->wpipe[0] = -1;
+  retVal->wpipe[1] = -1;
+  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
+#endif
 #ifdef DAUTH_SUPPORT
   retVal->digest_auth_rand_size = 0;
   retVal->digest_auth_random = NULL;
@@ -2464,12 +2517,15 @@
   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)
@@ -2548,6 +2604,18 @@
 #endif
   pthread_mutex_destroy (&daemon->per_ip_connection_mutex);
   pthread_mutex_destroy (&daemon->cleanup_connection_mutex);
+
+#ifndef HAVE_LISTEN_SHUTDOWN
+  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]);
+    }
+#endif
+
   free (daemon);
 }
 

Modified: libmicrohttpd/src/daemon/internal.h
===================================================================
--- libmicrohttpd/src/daemon/internal.h 2011-09-21 07:44:10 UTC (rev 16979)
+++ libmicrohttpd/src/daemon/internal.h 2011-09-21 07:47:03 UTC (rev 16980)
@@ -896,7 +896,14 @@
    */
   int socket_fd;
 
+#ifndef HAVE_LISTEN_SHUTDOWN
   /**
+   * Pipe we use to signal shutdown.
+   */
+  int wpipe[2];
+#endif
+
+  /**
    * Are we shutting down?
    */
   int shutdown;