found another race, just with partial work-around for now; also init errno in all cases

3 files changed, 40 insertions, 23 deletions

diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index bc26b53f..a8997d87 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -775,6 +775,7 @@ MHD_get_fdset2 (struct MHD_Daemon *daemon,
   struct MHD_Connection *pos;
   struct MHD_Connection *posn;
   int result = MHD_YES;
+  MHD_socket ls;
 
   if ( (NULL == daemon) ||
        (NULL == read_fd_set) ||
@@ -795,8 +796,9 @@ MHD_get_fdset2 (struct MHD_Daemon *daemon,
                                  fd_setsize) ? MHD_YES : MHD_NO;
     }
 #endif
-  if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
-       (! MHD_add_to_fd_set_ (daemon->socket_fd,
+  ls = daemon->socket_fd;
+  if ( (MHD_INVALID_SOCKET != ls) &&
+       (! MHD_add_to_fd_set_ (ls,
                               read_fd_set,
                               max_fd,
                               fd_setsize)) )
@@ -2222,6 +2224,7 @@ internal_add_connection (struct MHD_Daemon *daemon,
           gnutls_certificate_server_set_request (connection->tls_session,
                                                  GNUTLS_CERT_REQUEST);
 #else  /* ! HTTPS_SUPPORT */
+      eno = EINVAL;
       goto cleanup;
 #endif /* ! HTTPS_SUPPORT */
     }
@@ -2688,7 +2691,7 @@ MHD_accept_connection (struct MHD_Daemon *daemon)
       /* This could be a common occurance with multiple worker threads */
       if ( (MHD_SCKT_ERR_IS_ (err,
                               MHD_SCKT_EINVAL_)) &&
-           (MHD_INVALID_SOCKET == daemon->socket_fd) )
+           (MHD_INVALID_SOCKET == fd) )
         return MHD_NO; /* can happen during shutdown */
       if (MHD_SCKT_ERR_IS_DISCNN_BEFORE_ACCEPT_(err))
         return MHD_NO; /* do not print error if client just disconnected early */
@@ -3105,6 +3108,7 @@ MHD_select (struct MHD_Daemon *daemon,
   struct timeval *tv;
   MHD_UNSIGNED_LONG_LONG ltimeout;
   int err_state;
+  MHD_socket ls;
 
   timeout.tv_sec = 0;
   timeout.tv_usec = 0;
@@ -3140,8 +3144,8 @@ MHD_select (struct MHD_Daemon *daemon,
   else
     {
       /* accept only, have one thread per connection */
-      if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
-           (! MHD_add_to_fd_set_ (daemon->socket_fd,
+      if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
+           (! MHD_add_to_fd_set_ (ls,
                                   &rs,
                                   &maxsock,
                                   FD_SETSIZE)) )
@@ -3163,9 +3167,9 @@ MHD_select (struct MHD_Daemon *daemon,
       /* fdset limit reached, new connections
          cannot be handled. Remove listen socket FD
          from fdset and retry to add ITC FD. */
-      if (MHD_INVALID_SOCKET != daemon->socket_fd)
+      if (MHD_INVALID_SOCKET != (ls = daemon->socket_fd))
         {
-          FD_CLR (daemon->socket_fd,
+          FD_CLR (ls,
                   &rs);
           if (! MHD_add_to_fd_set_ (MHD_itc_r_fd_(daemon->itc),
                                     &rs,
@@ -3189,13 +3193,13 @@ MHD_select (struct MHD_Daemon *daemon,
      the shutdown OR the termination of an existing connection; so
      only do this optimization if we have a signaling ITC in
      place. */
-  if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
+  if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
        (MHD_ITC_IS_VALID_(daemon->itc)) &&
        (0 != (daemon->options & MHD_USE_ITC)) &&
        ( (daemon->connections == daemon->connection_limit) ||
          (daemon->at_limit) ) )
     {
-      FD_CLR (daemon->socket_fd,
+      FD_CLR (ls,
               &rs);
     }
   tv = NULL;
@@ -3287,6 +3291,7 @@ MHD_poll_all (struct MHD_Daemon *daemon,
     int poll_listen;
     int poll_itc_idx;
     struct pollfd *p;
+    MHD_socket ls;
 
     p = MHD_calloc_ ((2 + num_connections), sizeof (struct pollfd));
     if (NULL == p)
@@ -3300,12 +3305,12 @@ MHD_poll_all (struct MHD_Daemon *daemon,
       }
     poll_server = 0;
     poll_listen = -1;
-    if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
+    if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
          (daemon->connections < daemon->connection_limit) &&
          (! daemon->at_limit) )
       {
         /* only listen if we are not at the connection limit */
-        p[poll_server].fd = daemon->socket_fd;
+        p[poll_server].fd = ls;
         p[poll_server].events = POLLIN;
         p[poll_server].revents = 0;
         poll_listen = (int) poll_server;
@@ -3503,6 +3508,7 @@ MHD_poll_listen_socket (struct MHD_Daemon *daemon,
   unsigned int poll_count;
   int poll_listen;
   int poll_itc_idx;
+  MHD_socket ls;
 
   memset (&p,
           0,
@@ -3510,9 +3516,9 @@ MHD_poll_listen_socket (struct MHD_Daemon *daemon,
   poll_count = 0;
   poll_listen = -1;
   poll_itc_idx = -1;
-  if (MHD_INVALID_SOCKET != daemon->socket_fd)
+  if (MHD_INVALID_SOCKET != (ls = daemon->socket_fd))
     {
-      p[poll_count].fd = daemon->socket_fd;
+      p[poll_count].fd = ls;
       p[poll_count].events = POLLIN;
       p[poll_count].revents = 0;
       poll_listen = poll_count;
@@ -3701,6 +3707,7 @@ MHD_epoll (struct MHD_Daemon *daemon,
   MHD_UNSIGNED_LONG_LONG timeout_ll;
   int num_events;
   unsigned int i;
+  MHD_socket ls;
 #if defined(HTTPS_SUPPORT) && defined(UPGRADE_SUPPORT)
   int run_upgraded = MHD_NO;
 #endif /* HTTPS_SUPPORT && UPGRADE_SUPPORT */
@@ -3709,7 +3716,7 @@ MHD_epoll (struct MHD_Daemon *daemon,
     return MHD_NO; /* we're down! */
   if (daemon->shutdown)
     return MHD_NO;
-  if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
+  if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
        (daemon->connections < daemon->connection_limit) &&
        (MHD_NO == daemon->listen_socket_in_epoll) &&
        (! daemon->at_limit) )
@@ -3718,7 +3725,7 @@ MHD_epoll (struct MHD_Daemon *daemon,
       event.data.ptr = daemon;
       if (0 != epoll_ctl (daemon->epoll_fd,
                           EPOLL_CTL_ADD,
-                          daemon->socket_fd,
+                          ls,
                           &event))
         {
 #ifdef HAVE_MESSAGES
@@ -3759,7 +3766,7 @@ MHD_epoll (struct MHD_Daemon *daemon,
          for event loop for now */
       if (0 != epoll_ctl (daemon->epoll_fd,
                           EPOLL_CTL_DEL,
-                          daemon->socket_fd,
+                          ls,
                           NULL))
         MHD_PANIC (_("Failed to remove listen FD from epoll set\n"));
       daemon->listen_socket_in_epoll = MHD_NO;
@@ -4141,7 +4148,7 @@ MHD_start_daemon (unsigned int flags,
  * returned socket; however, if MHD is run using threads (anything but
  * external select mode), socket will be removed from existing threads
  * with some delay and it must not be closed while it's in use. To make
- * sure that socket is not used anymore, call #MHD_stop_daemon.
+ * sure that the socket is not used anymore, call #MHD_stop_daemon.
  *
  * Note that some thread modes require the caller to have passed
  * #MHD_USE_ITC when using this API.  If this daemon is
@@ -4196,6 +4203,9 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon)
               MHD_PANIC (_("Failed to signal quiesce via inter-thread communication channel"));
           }
       }
+  /* FIXME: This creates a race with the rest of the code.
+     We may be adding the FD to the epoll-set concurrently
+     in another thread! So we DO need to lock (yuck yuck). */
   daemon->socket_fd = MHD_INVALID_SOCKET;
 #ifdef EPOLL_SUPPORT
   if ( (0 != (daemon->options & MHD_USE_EPOLL)) &&
@@ -4698,6 +4708,7 @@ static int
 setup_epoll_to_listen (struct MHD_Daemon *daemon)
 {
   struct epoll_event event;
+  MHD_socket ls;
 
   daemon->epoll_fd = setup_epoll_fd (daemon);
   if (-1 == daemon->epoll_fd)
@@ -4710,13 +4721,13 @@ setup_epoll_to_listen (struct MHD_Daemon *daemon)
          return MHD_NO;
     }
 #endif /* HTTPS_SUPPORT && UPGRADE_SUPPORT */
-  if (MHD_INVALID_SOCKET == daemon->socket_fd)
+  if (MHD_INVALID_SOCKET == (ls = daemon->socket_fd))
     return MHD_YES; /* non-listening daemon */
   event.events = EPOLLIN;
   event.data.ptr = daemon;
   if (0 != epoll_ctl (daemon->epoll_fd,
                       EPOLL_CTL_ADD,
-                      daemon->socket_fd,
+                      ls,
                       &event))
     {
 #ifdef HAVE_MESSAGES
diff --git a/src/testcurl/test_get_response_cleanup.c b/src/testcurl/test_get_response_cleanup.c
index 26b2ecda..2833899c 100644
--- a/src/testcurl/test_get_response_cleanup.c
+++ b/src/testcurl/test_get_response_cleanup.c
@@ -77,6 +77,7 @@ fork_curl (const char *url)
   _exit (-1);
 }
 
+
 static void
 kill_curl (pid_t pid)
 {
@@ -97,6 +98,7 @@ push_callback (void *cls, uint64_t pos, char *buf, size_t max)
   return 1;
 }
 
+
 static void
 push_free_callback (void *cls)
 {
@@ -165,19 +167,20 @@ testInternalGet ()
   return 0;
 }
 
+
 static int
 testMultithreadedGet ()
 {
   struct MHD_Daemon *d;
   pid_t curl;
 
+  ok = 1;
   d = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION | MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_ERROR_LOG,
                         1081, NULL, NULL, &ahc_echo, "GET",
                         MHD_OPTION_CONNECTION_TIMEOUT, (unsigned int) 2,
                         MHD_OPTION_END);
   if (d == NULL)
     return 16;
-  ok = 1;
   //fprintf (stderr, "Forking cURL!\n");
   curl = fork_curl ("http://127.0.0.1:1081/");
   sleep (1);
@@ -201,18 +204,19 @@ testMultithreadedGet ()
   return 0;
 }
 
+
 static int
 testMultithreadedPoolGet ()
 {
   struct MHD_Daemon *d;
   pid_t curl;
 
+  ok = 1;
   d = MHD_start_daemon (MHD_USE_INTERNAL_POLLING_THREAD | MHD_USE_ERROR_LOG,
                         1081, NULL, NULL, &ahc_echo, "GET",
                         MHD_OPTION_THREAD_POOL_SIZE, CPU_COUNT, MHD_OPTION_END);
   if (d == NULL)
     return 64;
-  ok = 1;
   curl = fork_curl ("http://127.0.0.1:1081/");
   sleep (1);
   kill_curl (curl);
@@ -224,6 +228,7 @@ testMultithreadedPoolGet ()
   return 0;
 }
 
+
 static int
 testExternalGet ()
 {
@@ -236,6 +241,7 @@ testExternalGet ()
   struct timeval tv;
   pid_t curl;
 
+  ok = 1;
   d = MHD_start_daemon (MHD_USE_ERROR_LOG,
                         1082, NULL, NULL, &ahc_echo, "GET", MHD_OPTION_END);
   if (d == NULL)
diff --git a/src/testcurl/test_quiesce_stream.c b/src/testcurl/test_quiesce_stream.c
index 7885a048..38263e07 100644
--- a/src/testcurl/test_quiesce_stream.c
+++ b/src/testcurl/test_quiesce_stream.c
@@ -61,7 +61,7 @@ resume_connection (void *arg)
 {
   struct MHD_Connection *connection = arg;
 
-  fprintf (stderr, "Calling resume\n");
+  /* fprintf (stderr, "Calling resume\n"); */
   MHD_resume_connection (connection);
   return NULL;
 }
@@ -72,7 +72,7 @@ suspend_connection (struct MHD_Connection *connection)
 {
   pthread_t thread_id;
 
-  fprintf (stderr, "Calling suspend\n");
+  /* fprintf (stderr, "Calling suspend\n"); */
   MHD_suspend_connection (connection);
   int status = pthread_create (&thread_id,
                                NULL,