[libmicrohttpd] MHD_handle_connection race leads to deadlock in MHD_stop_daemon

[Mike Crowe]

I've found a difficult to reproduce deadlock in v0.4.2 when calling
MHD_stop_daemon while a connection is active. I narrowed it down to
MHD_handle_connection racing against MHD_cleanup_connections. It can
be reproduced much more easily by inserting a sleep just before the
call to SELECT in MHD_handle_connection. I'm using
MHD_USE_THREAD_PER_CONNECTION.

The problem appears to occur when MHD_stop_daemon sets
daemon->shutdown and sends SIGALRM signal to the thread running inside
MHD_handle_connection while it is between the top of the loop where
con->daemon->shutdown is checked and the call to SELECT. The signal
has no persistent effect and the thread ends up blocked forever inside
select.

The only workarounds I can come up with are using an explicit pipe to
wake the thread (which is stateful so it can be relied upon to wake
the select) or to force the socket closed so that the select wakes up
(but this sounds rather more dangerous and is racey if anyone else is
opening file descriptors.)

Here's a patch which reproduces the problem under 'make check'. I'm
running on Debian GNU/Linux 5.0 on amd64 but I've also reproduced the
problem on i386.

I can clean up the patch to make it suitable for checking in but the
chances of hitting the problem without the sleep in
MHD_handle_connection are very small.

Index: src/daemon/daemon.c
===================================================================
--- src/daemon/daemon.c (revision 9153)
+++ src/daemon/daemon.c (working copy)
@@ -487,6 +487,7 @@
           timeout = 1;
           tv.tv_sec = 0;
       }
+      usleep(1000000);
       num_ready = SELECT (max + 1,
                           &rs, &ws, &es, (timeout != 0) ? &tv : NULL);
       if (num_ready < 0)
Index: src/testcurl/daemontest_get.c
===================================================================
--- src/testcurl/daemontest_get.c       (revision 9153)
+++ src/testcurl/daemontest_get.c       (working copy)
@@ -434,7 +434,48 @@
   return 0;
 }
 
+static int
+testStopRace ()
+{
+    struct sockaddr_in sin;
+    int fd;
+    struct MHD_Daemon *d;
+    
+    d = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG,
+                         1081, NULL, NULL, &ahc_echo, "GET", MHD_OPTION_END);
+    if (d == NULL)
+       return 16;
+    
+    fd = socket(PF_INET, SOCK_STREAM, 0);
+    if (fd < 0)
+    {
+       fprintf(stderr, "socket: %m\n");
+       return 256;
+    }
+    
+    memset(&sin, 0, sizeof(sin));
+    sin.sin_family = AF_INET;
+    sin.sin_port = htons(1081);
+    sin.sin_addr.s_addr = htonl(0x7f000001);
+    
+    if (connect(fd, (struct sockaddr *)(&sin), sizeof(sin)) < 0)
+    {
+       fprintf(stderr, "connect: %m\n");
+       return 512;
+    }
+    
+    printf("Waiting\n");
+    // Let the thread get going.
+    usleep(500000);
+    
+    printf("Stopping daemon\n");
+    MHD_stop_daemon (d);
 
+    close(fd);
+    
+    printf("good\n");
+    return 0;
+}
 
 int
 main (int argc, char *const *argv)
@@ -444,6 +485,7 @@
   oneone = NULL != strstr (argv[0], "11");
   if (0 != curl_global_init (CURL_GLOBAL_WIN32))
     return 2;
+  errorCount += testStopRace ();
   errorCount += testInternalGet ();
   errorCount += testMultithreadedGet ();
   errorCount += testMultithreadedPoolGet ();

Thanks.

Mike.