gnunet-svn
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: gnunet
Subject: [GNUnet-SVN] r15731 - in libmicrohttpd: . src/daemon
Date: Tue, 21 Jun 2011 15:00:30 +0200

Author: grothoff
Date: 2011-06-21 15:00:30 +0200 (Tue, 21 Jun 2011)
New Revision: 15731

Modified:
   libmicrohttpd/ChangeLog
   libmicrohttpd/src/daemon/connection.c
   libmicrohttpd/src/daemon/connection_https.c
   libmicrohttpd/src/daemon/daemon.c
   libmicrohttpd/src/daemon/internal.h
Log:
fixing race condition, minor leak, O(n) shutdown is now O(1)

Modified: libmicrohttpd/ChangeLog
===================================================================
--- libmicrohttpd/ChangeLog     2011-06-21 12:46:43 UTC (rev 15730)
+++ libmicrohttpd/ChangeLog     2011-06-21 13:00:30 UTC (rev 15731)
@@ -1,3 +1,10 @@
+Tue Jun 21 13:54:59 CEST 2011
+       Fixing tiny memory leak in SSL code from 'gnutls_priority_init'.
+       Fixing data race between code doing connection shutdown and
+       connection cleanup.
+       Changing code to reduce connection cleanup cost from O(n) to O(1). 
+       Cleaning up logging code around 'connection_close_error'. -CG
+
 Sat Jun 11 13:05:12 CEST 2011
        Replacing use of sscanf by strtoul (#1688). -CG/bplant
 

Modified: libmicrohttpd/src/daemon/connection.c
===================================================================
--- libmicrohttpd/src/daemon/connection.c       2011-06-21 12:46:43 UTC (rev 
15730)
+++ libmicrohttpd/src/daemon/connection.c       2011-06-21 13:00:30 UTC (rev 
15731)
@@ -275,6 +275,7 @@
               strlen (HTTP_100_CONTINUE)));
 }
 
+
 /**
  * Close the given connection and give the
  * specified termination code to the user.
@@ -283,31 +284,52 @@
 MHD_connection_close (struct MHD_Connection *connection,
                       enum MHD_RequestTerminationCode termination_code)
 {
+  struct MHD_Daemon *daemon;
+
+  daemon = connection->daemon;
   SHUTDOWN (connection->socket_fd, SHUT_RDWR);
-  CLOSE (connection->socket_fd);
-  connection->socket_fd = -1;
   connection->state = MHD_CONNECTION_CLOSED;
-  if ( (NULL != connection->daemon->notify_completed) &&
+  if ( (NULL != daemon->notify_completed) &&
        (MHD_YES == connection->client_aware) )
-    connection->daemon->notify_completed (connection->daemon->
-                                         notify_completed_cls, connection,
-                                         &connection->client_context,
-                                         termination_code);
+    daemon->notify_completed (daemon->notify_completed_cls, 
+                             connection,
+                             &connection->client_context,
+                             termination_code);
   connection->client_aware = MHD_NO;
 }
 
+
 /**
  * A serious error occured, close the
  * connection (and notify the application).
+ *
+ * @param connection connection to close with error
+ * @param emsg error message (can be NULL)
  */
 static void
-connection_close_error (struct MHD_Connection *connection)
+connection_close_error (struct MHD_Connection *connection,
+                       const char *emsg)
 {
+#if HAVE_MESSAGES
+  if (NULL != emsg)
+    MHD_DLOG (connection->daemon, emsg);
+#endif
   MHD_connection_close (connection, MHD_REQUEST_TERMINATED_WITH_ERROR);
 }
 
 
 /**
+ * Macro to only include error message in call to
+ * "connection_close_error" if we have HAVE_MESSAGES.
+ */
+#if HAVE_MESSAGES
+#define CONNECTION_CLOSE_ERROR(c, emsg) connection_close_error (c, emsg)
+#else
+#define CONNECTION_CLOSE_ERROR(c, emsg) connection_close_error (c, NULL)
+#endif
+
+
+/**
  * Prepare the response buffer of this connection for
  * sending.  Assumes that the response mutex is
  * already held.  If the transmission is complete,
@@ -357,16 +379,12 @@
   if ( (ret == MHD_CONTENT_READER_END_OF_STREAM) ||
        (ret == MHD_CONTENT_READER_END_WITH_ERROR) )
     {
-      /* either error or http 1.0 transfer, close
-         socket! */
-#if DEBUG_CLOSE
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon, 
-               "Closing connection (end of response or error)\n");
-#endif
-#endif
+      /* either error or http 1.0 transfer, close socket! */
       response->total_size = connection->response_write_position;
-      connection_close_error (connection);
+      CONNECTION_CLOSE_ERROR (connection,
+                             (ret == MHD_CONTENT_READER_END_OF_STREAM) 
+                             ? "Closing connection (end of response)\n"
+                             : "Closing connection (stream error)\n");
       return MHD_NO;
     }
   response->data_start = connection->response_write_position;
@@ -405,13 +423,8 @@
           if (size < 128)
             {
               /* not enough memory */
-#if DEBUG_CLOSE
-#if HAVE_MESSAGES
-              MHD_DLOG (connection->daemon,
-                        "Closing connection (out of memory)\n");
-#endif
-#endif
-              connection_close_error (connection);
+              CONNECTION_CLOSE_ERROR (connection,
+                                     "Closing connection (out of memory)\n");
               return MHD_NO;
             }
           buf = MHD_pool_allocate (connection->pool, size, MHD_NO);
@@ -445,14 +458,9 @@
   if (ret == MHD_CONTENT_READER_END_WITH_ERROR) 
     {
       /* error, close socket! */
-#if DEBUG_CLOSE
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon, 
-               "Closing connection (error generating response)\n");
-#endif
-#endif
       response->total_size = connection->response_write_position;
-      connection_close_error (connection);
+      CONNECTION_CLOSE_ERROR (connection,
+                             "Closing connection (error generating 
response)\n");
       return MHD_NO;
     }
   if (ret == MHD_CONTENT_READER_END_OF_STREAM) 
@@ -722,11 +730,8 @@
   if (MHD_NO == build_header_response (connection))
     {
       /* oops - close! */
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon,
-                "Closing connection (failed to create response header)\n");
-#endif
-      connection->state = MHD_CONNECTION_CLOSED;
+      CONNECTION_CLOSE_ERROR (connection,
+                             "Closing connection (failed to create response 
header)\n");
     }
   else
     {
@@ -788,11 +793,9 @@
     connection->pool = MHD_pool_create (connection->daemon->pool_size);
   if (connection->pool == NULL)
     {
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon, "Failed to create memory pool!\n");
-#endif
-      connection_close_error (connection);
-      return MHD_NO;
+      CONNECTION_CLOSE_ERROR (connection,
+                             "Failed to create memory pool!\n");
+      return MHD_YES;
     }
   fd = connection->socket_fd;
   p->fd = fd;
@@ -822,7 +825,8 @@
           if ((connection->read_closed) &&
               (connection->read_buffer_offset == 0))
             {
-              connection->state = MHD_CONNECTION_CLOSED;
+             CONNECTION_CLOSE_ERROR (connection, 
+                                     "Connection buffer to small for 
request\n");
               continue;
             }
           if ((connection->read_buffer_offset == connection->read_buffer_size)
@@ -883,7 +887,8 @@
              read buffer if needed, no size-check required */
           if (MHD_YES == connection->read_closed)
             {
-              connection->state = MHD_CONNECTION_CLOSED;
+             CONNECTION_CLOSE_ERROR (connection, 
+                                     "Connection closed while reading 
request\n");
               continue;
             }
           p->events |= MHD_POLL_ACTION_IN;
@@ -923,10 +928,7 @@
           EXTRA_CHECK (0);
           break;
         case MHD_CONNECTION_CLOSED:
-          if (connection->socket_fd != -1)
-            connection_close_error (connection);
           return MHD_YES;       /* do nothing, not even reading */
-
         default:
           EXTRA_CHECK (0);
         }
@@ -1227,11 +1229,8 @@
                                           &connection->client_context))
     {
       /* serious internal error, close connection */
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon,
-               "Internal application error, closing connection.\n");
-#endif
-      connection_close_error (connection);
+      CONNECTION_CLOSE_ERROR (connection, 
+                             "Internal application error, closing 
connection.\n");
       return;
     }
 }
@@ -1279,11 +1278,8 @@
               if (i == 0)
                 {
                   /* malformed encoding */
-#if HAVE_MESSAGES
-                  MHD_DLOG (connection->daemon,
-                            "Received malformed HTTP request (bad chunked 
encoding), closing connection.\n");
-#endif
-                  connection_close_error (connection);
+                  CONNECTION_CLOSE_ERROR (connection,
+                                         "Received malformed HTTP request (bad 
chunked encoding), closing connection.\n");
                   return;
                 }
               available -= i;
@@ -1334,11 +1330,8 @@
               if (malformed)
                 {
                   /* malformed encoding */
-#if HAVE_MESSAGES
-                  MHD_DLOG (connection->daemon,
-                            "Received malformed HTTP request (bad chunked 
encoding), closing connection.\n");
-#endif
-                  connection_close_error (connection);
+                  CONNECTION_CLOSE_ERROR (connection,
+                                         "Received malformed HTTP request (bad 
chunked encoding), closing connection.\n");
                   return;
                 }
               i++;
@@ -1390,11 +1383,8 @@
                                                &connection->client_context))
         {
           /* serious internal error, close connection */
-#if HAVE_MESSAGES
-          MHD_DLOG (connection->daemon,
-                    "Internal application error, closing connection.\n");
-#endif
-          connection_close_error (connection);
+         CONNECTION_CLOSE_ERROR (connection,
+                                 "Internal application error, closing 
connection.\n");
           return;
         }
       if (processed > used)
@@ -1458,7 +1448,7 @@
        MHD_DLOG (connection->daemon,
                  "Failed to receive data: %s\n", STRERROR (errno));
 #endif
-      connection_close_error (connection);
+      CONNECTION_CLOSE_ERROR (connection, NULL);
       return MHD_YES;
     }
   if (bytes_read == 0)
@@ -1505,7 +1495,7 @@
        MHD_DLOG (connection->daemon,
                  "Failed to send data: %s\n", STRERROR (errno));
 #endif
-      connection_close_error (connection);
+      CONNECTION_CLOSE_ERROR (connection, NULL);
       return MHD_YES;
     }
 #if DEBUG_SEND_DATA
@@ -1555,11 +1545,8 @@
   if (colon == NULL)
     {
       /* error in header line, die hard */
-#if HAVE_MESSAGES
-      MHD_DLOG (connection->daemon,
-                "Received malformed line (no colon), closing connection.\n");
-#endif
-      connection->state = MHD_CONNECTION_CLOSED;
+      CONNECTION_CLOSE_ERROR (connection, 
+                             "Received malformed line (no colon), closing 
connection.\n");
       return MHD_NO;
     }
   /* zero-terminate header */
@@ -1696,7 +1683,7 @@
                     "Failed to parse `%s' header `%s', closing connection.\n",
                     MHD_HTTP_HEADER_CONTENT_LENGTH, clen);
 #endif
-          connection->state = MHD_CONNECTION_CLOSED;
+         CONNECTION_CLOSE_ERROR (connection, NULL);
           return;
         }
       connection->remaining_upload_size = cval;
@@ -1726,15 +1713,15 @@
  * implementations (multithreaded, external select, internal select)
  * call this function to handle reads.
  *
- * @return MHD_YES if we should continue to process the
- *         connection (not dead yet), MHD_NO if it died
+ * @return always MHD_YES (we should continue to process the
+ *         connection)
  */
 int
 MHD_connection_handle_read (struct MHD_Connection *connection)
 {
   connection->last_activity = time (NULL);
   if (connection->state == MHD_CONNECTION_CLOSED)
-    return MHD_NO;
+    return MHD_YES;
   /* make sure "read" has a reasonable number of bytes
      in buffer to use per system call (if possible) */
   if (connection->read_buffer_offset + MHD_BUF_INC_SIZE >
@@ -1767,9 +1754,7 @@
             }
           break;
         case MHD_CONNECTION_CLOSED:
-          if (connection->socket_fd != -1)
-            connection_close_error (connection);
-          return MHD_NO;
+          return MHD_YES;
         default:
           /* shrink read buffer to how much is actually used */
           MHD_pool_reallocate (connection->pool,
@@ -1783,14 +1768,15 @@
   return MHD_YES;
 }
 
+
 /**
  * This function was created to handle writes to sockets when it has
  * been determined that the socket can be written to. All
  * implementations (multithreaded, external select, internal select)
  * call this function
  *
- * @return MHD_YES if we should continue to process the
- *         connection (not dead yet), MHD_NO if it died
+ * @return always MHD_YES (we should continue to process the
+ *         connection)
  */
 int
 MHD_connection_handle_write (struct MHD_Connection *connection)
@@ -1828,8 +1814,8 @@
               MHD_DLOG (connection->daemon,
                         "Failed to send data: %s\n", STRERROR (errno));
 #endif
-              connection_close_error (connection);
-              return MHD_NO;
+             CONNECTION_CLOSE_ERROR (connection, NULL);
+              return MHD_YES;
             }
 #if DEBUG_SEND_DATA
           FPRINTF (stderr,
@@ -1889,8 +1875,8 @@
               MHD_DLOG (connection->daemon,
                         "Failed to send data: %s\n", STRERROR (errno));
 #endif
-              connection_close_error (connection);
-              return MHD_NO;
+             CONNECTION_CLOSE_ERROR (connection, NULL);
+              return MHD_YES;
             }
           connection->response_write_position += ret;
           if (connection->response_write_position ==
@@ -1920,16 +1906,14 @@
           EXTRA_CHECK (0);
           break;
         case MHD_CONNECTION_CLOSED:
-          if (connection->socket_fd != -1)
-            connection_close_error (connection);
-          return MHD_NO;
+          return MHD_YES;
         case MHD_TLS_CONNECTION_INIT:
           EXTRA_CHECK (0);
           break;
         default:
           EXTRA_CHECK (0);
-          connection_close_error (connection);
-          return MHD_NO;
+         CONNECTION_CLOSE_ERROR (connection, "Internal error\n");
+          return MHD_YES;
         }
       break;
     }
@@ -1948,6 +1932,7 @@
 int
 MHD_connection_handle_idle (struct MHD_Connection *connection)
 {
+  struct MHD_Daemon *daemon;
   unsigned int timeout;
   const char *end;
   char *line;
@@ -1968,13 +1953,14 @@
                 continue;
               if (connection->read_closed)
                 {
-                  connection->state = MHD_CONNECTION_CLOSED;
+                 CONNECTION_CLOSE_ERROR (connection, 
+                                         "Connection closed while reading 
request\n");
                   continue;
                 }
               break;
             }
           if (MHD_NO == parse_initial_message_line (connection, line))
-            connection->state = MHD_CONNECTION_CLOSED;
+            CONNECTION_CLOSE_ERROR (connection, NULL);
           else
             connection->state = MHD_CONNECTION_URL_RECEIVED;
           continue;
@@ -1986,7 +1972,8 @@
                 continue;
               if (connection->read_closed)
                 {
-                  connection->state = MHD_CONNECTION_CLOSED;
+                 CONNECTION_CLOSE_ERROR (connection, 
+                                         "Connection closed while reading 
request\n");
                   continue;
                 }
               break;
@@ -2013,7 +2000,8 @@
                 continue;
               if (connection->read_closed)
                 {
-                  connection->state = MHD_CONNECTION_CLOSED;
+                 CONNECTION_CLOSE_ERROR (connection, 
+                                         "Connection closed while reading 
request\n");
                   continue;
                 }
               break;
@@ -2088,7 +2076,8 @@
                 continue;
               if (connection->read_closed)
                 {
-                  connection->state = MHD_CONNECTION_CLOSED;
+                 CONNECTION_CLOSE_ERROR (connection, 
+                                         "Connection closed while reading 
request\n");
                   continue;
                 }
               break;
@@ -2115,7 +2104,8 @@
                 continue;
               if (connection->read_closed)
                 {
-                  connection->state = MHD_CONNECTION_CLOSED;
+                 CONNECTION_CLOSE_ERROR (connection, 
+                                         "Connection closed while reading 
request\n");
                   continue;
                 }
               break;
@@ -2138,11 +2128,8 @@
           if (MHD_NO == build_header_response (connection))
             {
               /* oops - close! */
-#if HAVE_MESSAGES
-              MHD_DLOG (connection->daemon,
-                        "Closing connection (failed to create response 
header)\n");
-#endif
-              connection->state = MHD_CONNECTION_CLOSED;
+             CONNECTION_CLOSE_ERROR (connection, 
+                                     "Closing connection (failed to create 
response header)\n");
               continue;
             }
           connection->state = MHD_CONNECTION_HEADERS_SENDING;
@@ -2253,7 +2240,7 @@
               (0 != strcasecmp (MHD_HTTP_VERSION_1_1, connection->version)))
             {
               /* http 1.0, version-less requests cannot be pipelined */
-              connection->state = MHD_CONNECTION_CLOSED;
+              MHD_connection_close (connection, 
MHD_REQUEST_TERMINATED_COMPLETED_OK);
               MHD_pool_destroy (connection->pool);
               connection->pool = NULL;
               connection->read_buffer = NULL;
@@ -2271,9 +2258,28 @@
             }
           continue;
         case MHD_CONNECTION_CLOSED:
-          if (connection->socket_fd != -1)
-            connection_close_error (connection);
-          break;
+         daemon = connection->daemon;
+         DLL_remove (daemon->connections_head,
+                     daemon->connections_tail,
+                     connection);
+         if (0 != pthread_mutex_lock(&daemon->cleanup_connection_mutex))
+           {
+#if HAVE_MESSAGES
+             MHD_DLOG (daemon, "Failed to acquire cleanup mutex\n");
+#endif
+             abort();
+           }
+         DLL_insert (daemon->cleanup_head,
+                     daemon->cleanup_tail,
+                     connection);
+         if (0 != pthread_mutex_unlock(&daemon->cleanup_connection_mutex))
+           {
+#if HAVE_MESSAGES
+             MHD_DLOG (daemon, "Failed to release cleanup mutex\n");
+#endif
+             abort();
+           }
+         return MHD_NO;
         default:
           EXTRA_CHECK (0);
           break;
@@ -2281,12 +2287,11 @@
       break;
     }
   timeout = connection->daemon->connection_timeout;
-  if ((connection->socket_fd != -1) &&
-      (timeout != 0) &&
-      (timeout <= (time (NULL) - connection->last_activity)) )
+  if ( (timeout != 0) &&
+       (timeout <= (time (NULL) - connection->last_activity)) )
     {
       MHD_connection_close (connection, 
MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
-      return MHD_NO;
+      return MHD_YES;
     }
   return MHD_YES;
 }

Modified: libmicrohttpd/src/daemon/connection_https.c
===================================================================
--- libmicrohttpd/src/daemon/connection_https.c 2011-06-21 12:46:43 UTC (rev 
15730)
+++ libmicrohttpd/src/daemon/connection_https.c 2011-06-21 13:00:30 UTC (rev 
15731)
@@ -33,23 +33,6 @@
 #include "reason_phrase.h"
 #include <gnutls/gnutls.h>
 
-/**
- * This function is called once a secure connection has been marked
- * for closure.
- *
- * NOTE: Some code duplication with connection_close_error
- * in connection.c
- *
- * @param connection: the connection to close
- * @param termination_code: the termination code with which the notify 
completed callback function is called.
- */
-static void
-MHD_tls_connection_close (struct MHD_Connection *connection,
-                          enum MHD_RequestTerminationCode termination_code)
-{
-  gnutls_bye (connection->tls_session, GNUTLS_SHUT_RDWR);
-  MHD_connection_close (connection, termination_code);
-}
 
 
 /**
@@ -65,9 +48,8 @@
  * Application data is forwarded to the underlying daemon for
  * processing.
  *
- * @param connection : the source connection
- * @return MHD_YES if we should continue to process the
- *         connection (not dead yet), MHD_NO if it died
+ * @param connection the source connection
+ * @return always MHD_YES (we should continue to process the connection)
  */
 static int
 MHD_tls_connection_handle_read (struct MHD_Connection *connection)
@@ -95,9 +77,9 @@
       MHD_DLOG (connection->daemon,
                "Error: received handshake message out of context\n");
 #endif
-      MHD_tls_connection_close (connection,
-                               MHD_REQUEST_TERMINATED_WITH_ERROR);
-      return MHD_NO;
+      MHD_connection_close (connection,
+                           MHD_REQUEST_TERMINATED_WITH_ERROR);
+      return MHD_YES;
     }
   return MHD_connection_handle_read (connection);
 }
@@ -109,8 +91,7 @@
  * will forward all write requests to the underlying daemon unless
  * the connection has been marked for closing.
  *
- * @return MHD_connection_handle_write() if we should continue to
- *         process the connection (not dead yet), MHD_NO if it died
+ * @return always MHD_YES (we should continue to process the connection)
  */
 static int
 MHD_tls_connection_handle_write (struct MHD_Connection *connection)
@@ -142,9 +123,9 @@
       MHD_DLOG (connection->daemon,
                "Error: received handshake message out of context\n");
 #endif
-      MHD_tls_connection_close (connection,
-                               MHD_REQUEST_TERMINATED_WITH_ERROR);
-      return MHD_NO;
+      MHD_connection_close (connection,
+                           MHD_REQUEST_TERMINATED_WITH_ERROR);
+      return MHD_YES;
     }
   return MHD_connection_handle_write (connection);
 }
@@ -170,13 +151,9 @@
             __FUNCTION__, MHD_state_to_string (connection->state));
 #endif
   timeout = connection->daemon->connection_timeout;
-  if ((connection->socket_fd != -1) && (timeout != 0)
-      && (time (NULL) - timeout > connection->last_activity))
-    {
-      MHD_tls_connection_close (connection,
-                                MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
-      return MHD_NO;
-    }
+  if ( (timeout != 0) && (time (NULL) - timeout > connection->last_activity))
+    MHD_connection_close (connection,
+                         MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
   switch (connection->state)
     {
       /* on newly created connections we might reach here before any reply has 
been received */
@@ -184,14 +161,12 @@
       return MHD_YES;
       /* close connection if necessary */
     case MHD_CONNECTION_CLOSED:
-      if (connection->socket_fd != -1)
-        MHD_tls_connection_close (connection,
-                                  MHD_REQUEST_TERMINATED_COMPLETED_OK);
-      return MHD_NO;
+      gnutls_bye (connection->tls_session, GNUTLS_SHUT_RDWR);
+      return MHD_connection_handle_idle (connection);
     default:
       if ( (0 != gnutls_record_check_pending (connection->tls_session)) &&
           (MHD_YES != MHD_tls_connection_handle_read (connection)) )
-       return MHD_NO;
+       return MHD_YES;
       return MHD_connection_handle_idle (connection);
     }
   return MHD_YES;

Modified: libmicrohttpd/src/daemon/daemon.c
===================================================================
--- libmicrohttpd/src/daemon/daemon.c   2011-06-21 12:46:43 UTC (rev 15730)
+++ libmicrohttpd/src/daemon/daemon.c   2011-06-21 13:00:30 UTC (rev 15731)
@@ -150,7 +150,7 @@
 };
 
 /**
- * Lock shared structure for IP connection counts
+ * Lock shared structure for IP connection counts and connection DLLs.
  *
  * @param daemon handle to daemon where lock is
  */
@@ -167,7 +167,7 @@
 }
 
 /**
- * Unlock shared structure for IP connection counts
+ * Unlock shared structure for IP connection counts and connection DLLs.
  *
  * @param daemon handle to daemon where lock is
  */
@@ -495,6 +495,7 @@
 }
 #endif
 
+
 /**
  * Obtain the select sets for this daemon.
  *
@@ -513,7 +514,8 @@
                fd_set * read_fd_set,
                fd_set * write_fd_set, fd_set * except_fd_set, int *max_fd)
 {
-  struct MHD_Connection *con_itr;
+  struct MHD_Connection *pos;
+  struct MHD_Connection *next;
   int fd;
 
   if ((daemon == NULL) || (read_fd_set == NULL) || (write_fd_set == NULL)
@@ -528,15 +530,15 @@
   if ((*max_fd) < fd) 
     *max_fd = fd;
 
-  con_itr = daemon->connections;
-  while (con_itr != NULL)
+  next = daemon->connections_head;
+  while (NULL != (pos = next))
     {
-      if (MHD_YES != MHD_connection_get_fdset (con_itr,
+      next = pos->next;
+      if (MHD_YES != MHD_connection_get_fdset (pos,
                                                read_fd_set,
                                                write_fd_set,
                                                except_fd_set, max_fd))
         return MHD_NO;
-      con_itr = con_itr->next;
     }
 #if DEBUG_CONNECT
   MHD_DLOG (daemon, "Maximum socket in select set: %d\n", *max_fd);
@@ -572,7 +574,8 @@
 #endif
 
   timeout = con->daemon->connection_timeout;
-  while ((!con->daemon->shutdown) && (con->socket_fd != -1)) {
+  while ( (!con->daemon->shutdown) && (con->state != MHD_CONNECTION_CLOSED) ) 
+    {
       tvp = NULL;
       if (timeout > 0)
        {
@@ -624,17 +627,19 @@
              break;
            }
          /* call appropriate connection handler if necessary */
-         if ((con->socket_fd != -1) && (FD_ISSET (con->socket_fd, &rs)))
+         if (FD_ISSET (con->socket_fd, &rs))
            con->read_handler (con);
-         if ((con->socket_fd != -1) && (FD_ISSET (con->socket_fd, &ws)))
+         if (FD_ISSET (con->socket_fd, &ws))
            con->write_handler (con);
-         if (con->socket_fd != -1)
-           con->idle_handler (con);
+         if (MHD_NO == con->idle_handler (con))
+           {
+             return NULL;
+           }
        }
 #ifdef HAVE_POLL_H
       else
        {
-         /* use poll */
+           /* use poll */
          memset(&mp, 0, sizeof (struct MHD_Pollfd));
          MHD_connection_get_pollfd(con, &mp);
          memset(&p, 0, sizeof (p));
@@ -666,21 +671,20 @@
 #endif
              break;
            }
-         if ( (con->socket_fd != -1) && 
-              (0 != (p[0].revents & POLLIN)) ) 
+         if (0 != (p[0].revents & POLLIN)) 
            con->read_handler (con);        
-         if ( (con->socket_fd != -1) && 
-              (0 != (p[0].revents & POLLOUT)) )
+         if (0 != (p[0].revents & POLLOUT)) 
            con->write_handler (con);        
-         if (con->socket_fd != -1)
-           con->idle_handler (con);
-         if ( (con->socket_fd != -1) &&
-              (0 != (p[0].revents & (POLLERR | POLLHUP))) )
+         if (0 != (p[0].revents & (POLLERR | POLLHUP))) 
            MHD_connection_close (con, MHD_REQUEST_TERMINATED_WITH_ERROR);      
+         if (MHD_NO == con->idle_handler (con))
+           {
+             return NULL; /* "instant" termination, 'con' no longer valid! */
+           }
        }
 #endif
-      }
-  if (con->socket_fd != -1)
+    }
+  if (con->state != MHD_CONNECTION_IN_CLEANUP)
     {
 #if DEBUG_CLOSE
 #if HAVE_MESSAGES
@@ -688,7 +692,9 @@
                 "Processing thread terminating, closing connection\n");
 #endif
 #endif
-      MHD_connection_close (con, MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
+      if (con->state != MHD_CONNECTION_CLOSED)
+       MHD_connection_close (con, MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
+      con->idle_handler (con);
     }
   return NULL;
 }
@@ -706,8 +712,12 @@
                    void *other, 
                    size_t i)
 {
-  if (connection->socket_fd == -1)
-    return -1;
+  if ( (connection->socket_fd == -1) ||
+       (connection->state == MHD_CONNECTION_CLOSED) )
+    {
+      errno = ENOTCONN;
+      return -1;
+    }
   if (0 != (connection->daemon->options & MHD_USE_SSL))
     return RECV (connection->socket_fd, other, i, MSG_NOSIGNAL);
   return RECV (connection->socket_fd, other, i, MSG_NOSIGNAL);
@@ -732,8 +742,12 @@
   off_t left;
   ssize_t ret;
 #endif
-  if (connection->socket_fd == -1)
-    return -1;
+  if ( (connection->socket_fd == -1) ||
+       (connection->state == MHD_CONNECTION_CLOSED) )
+    {
+      errno = ENOTCONN;
+      return -1;
+    }
   if (0 != (connection->daemon->options & MHD_USE_SSL))
     return SEND (connection->socket_fd, other, i, MSG_NOSIGNAL);
 #if LINUX
@@ -997,19 +1011,19 @@
          /* use non-blocking IO for gnutls */
          socket_set_nonblocking (connection->socket_fd);
        }
-      switch (connection->daemon->cred_type)
+      switch (daemon->cred_type)
         {
           /* set needed credentials for certificate authentication. */
         case GNUTLS_CRD_CERTIFICATE:
           gnutls_credentials_set (connection->tls_session,
                                  GNUTLS_CRD_CERTIFICATE,
-                                 connection->daemon->x509_cred);
+                                 daemon->x509_cred);
           break;
         default:
 #if HAVE_MESSAGES
           MHD_DLOG (connection->daemon,
                     "Failed to setup TLS credentials: unknown credential type 
%d\n",
-                    connection->daemon->cred_type);
+                    daemon->cred_type);
 #endif
           SHUTDOWN (client_socket, SHUT_RDWR);
           CLOSE (client_socket);
@@ -1059,8 +1073,10 @@
           return MHD_NO;
         }
     }
-  connection->next = daemon->connections;
-  daemon->connections = connection;
+  /* FIXME: race with removal operation! */
+  DLL_insert (daemon->connections_head,
+             daemon->connections_tail,
+             connection);
   daemon->max_connections--;
   return MHD_YES;  
 }
@@ -1113,10 +1129,11 @@
                             addr, addrlen);
 }
 
+
 /**
  * Free resources associated with all closed connections.
- * (destroy responses, free buffers, etc.).  A connection
- * is known to be closed if the socket_fd is -1.
+ * (destroy responses, free buffers, etc.).  All closed
+ * connections are kept in the "cleanup" doubly-linked list.
  *
  * @param daemon daemon to clean up
  */
@@ -1124,59 +1141,61 @@
 MHD_cleanup_connections (struct MHD_Daemon *daemon)
 {
   struct MHD_Connection *pos;
-  struct MHD_Connection *prev;
   void *unused;
   int rc;
 
-  pos = daemon->connections;
-  prev = NULL;
-  while (pos != NULL)
+  if (0 != pthread_mutex_lock(&daemon->cleanup_connection_mutex))
     {
-      if ((pos->socket_fd == -1) ||
-          (((0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
-            (daemon->shutdown) && (pos->socket_fd != -1))))
-        {
-          if (prev == NULL)
-            daemon->connections = pos->next;
-          else
-            prev->next = pos->next;
-          if (0 != (pos->daemon->options & MHD_USE_THREAD_PER_CONNECTION))
-            {
-
-              if (0 != (rc = pthread_join (pos->pid, &unused)))
-               {
 #if HAVE_MESSAGES
-                 MHD_DLOG (daemon, "Failed to join a thread: %s\n",
-                           STRERROR (rc));
+      MHD_DLOG (daemon, "Failed to acquire cleanup mutex\n");
 #endif
-                 abort();
-               }
-            }
-          MHD_pool_destroy (pos->pool);
+      abort();
+    }
+  while (NULL != (pos = daemon->cleanup_head))
+    {
+      DLL_remove (daemon->cleanup_head,
+                 daemon->cleanup_tail,
+                 pos);
+      if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+          (MHD_NO == pos->thread_joined) )
+       { 
+         if (0 != (rc = pthread_join (pos->pid, &unused)))
+           {
+#if HAVE_MESSAGES
+             MHD_DLOG (daemon, "Failed to join a thread: %s\n",
+                       STRERROR (rc));
+#endif
+             abort();
+           }
+       }
+      MHD_pool_destroy (pos->pool);
 #if HTTPS_SUPPORT
-          if (pos->tls_session != NULL)
-            gnutls_deinit (pos->tls_session);
+      if (pos->tls_session != NULL)
+       gnutls_deinit (pos->tls_session);
 #endif
-          MHD_ip_limit_del (daemon, (struct sockaddr*)pos->addr, 
pos->addr_len);
-         if (pos->response != NULL)
-           {
-             MHD_destroy_response (pos->response);
-             pos->response = NULL;
-           }
-          free (pos->addr);
-          free (pos);
-          daemon->max_connections++;
-          if (prev == NULL)
-            pos = daemon->connections;
-          else
-            pos = prev->next;
-          continue;
-        }
-      prev = pos;
-      pos = pos->next;
+      MHD_ip_limit_del (daemon, (struct sockaddr*)pos->addr, pos->addr_len);
+      if (pos->response != NULL)
+       {
+         MHD_destroy_response (pos->response);
+         pos->response = NULL;
+       }
+      if (-1 != pos->socket_fd)
+       CLOSE (pos->socket_fd);
+      if (NULL != pos->addr)
+       free (pos->addr);
+      free (pos);
+      daemon->max_connections++;
     }
+  if (0 != pthread_mutex_unlock(&daemon->cleanup_connection_mutex))
+    {
+#if HAVE_MESSAGES
+      MHD_DLOG (daemon, "Failed to release cleanup mutex\n");
+#endif
+      abort();
+    }
 }
 
+
 /**
  * Obtain timeout value for select for this daemon
  * (only needed if connection timeout is used).  The
@@ -1201,7 +1220,7 @@
   dto = daemon->connection_timeout;
   if (0 == dto)
     return MHD_NO;
-  pos = daemon->connections;
+  pos = daemon->connections_head;
   if (pos == NULL)
     return MHD_NO;              /* no connections */
   now = time (NULL);
@@ -1238,6 +1257,7 @@
            int may_block)
 {
   struct MHD_Connection *pos;
+  struct MHD_Connection *next;
   int num_ready;
   fd_set rs;
   fd_set ws;
@@ -1313,21 +1333,19 @@
   if (0 == (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
     {
       /* do not have a thread per connection, process all connections now */
-      pos = daemon->connections;
-      while (pos != NULL)
+      next = daemon->connections_head;
+      while (NULL != (pos = next))
         {
+         next = pos->next;
           ds = pos->socket_fd;
           if (ds != -1)
             {
-              /* TODO call con->read handler */
               if (FD_ISSET (ds, &rs))
                 pos->read_handler (pos);
-              if ((pos->socket_fd != -1) && (FD_ISSET (ds, &ws)))
+              if (FD_ISSET (ds, &ws))
                 pos->write_handler (pos);
-              if (pos->socket_fd != -1)
-                pos->idle_handler (pos);
+             pos->idle_handler (pos);
             }
-          pos = pos->next;
         }
     }
   return MHD_YES;
@@ -1349,9 +1367,10 @@
 {
   unsigned int num_connections;
   struct MHD_Connection *pos;
+  struct MHD_Connection *next;
 
   num_connections = 0;
-  pos = daemon->connections;
+  pos = daemon->connections_head;
   while (pos != NULL)
     {
       num_connections++;
@@ -1385,7 +1404,7 @@
       timeout = (ltimeout > INT_MAX) ? INT_MAX : (int) ltimeout;
     
     i = 0;
-    pos = daemon->connections;
+    pos = daemon->connections_head;
     while (pos != NULL)
       {
        memset(&mp, 0, sizeof (struct MHD_Pollfd));
@@ -1413,9 +1432,10 @@
     if (daemon->socket_fd < 0) 
       return MHD_YES; 
     i = 0;
-    pos = daemon->connections;
-    while (pos != NULL)
+    next = daemon->connections_head;
+    while (NULL != (pos = next))
       {
+       next = pos->next;
        /* first, sanity checks */
        if (i >= num_connections)
          break; /* connection list changed somehow, retry later ... */
@@ -1427,11 +1447,9 @@
        if (0 != (p[poll_server+i].revents & POLLIN)) 
          pos->read_handler (pos);
        if (0 != (p[poll_server+i].revents & POLLOUT)) 
-         pos->write_handler (pos);
-       if (pos->socket_fd != -1)
-         pos->idle_handler (pos);
+         pos->write_handler (pos);     
+       pos->idle_handler (pos);
        i++;
-       pos = pos->next;
       }
     if ( (1 == poll_server) &&
         (0 != (p[0].revents & POLLIN)) )
@@ -1729,18 +1747,22 @@
          daemon->cred_type = va_arg (ap, gnutls_credentials_type_t);
          break;
         case MHD_OPTION_HTTPS_PRIORITIES:
-         ret = gnutls_priority_init (&daemon->priority_cache,
-                                     pstr = va_arg (ap, const char*),
-                                     NULL);
+         if (daemon->options & MHD_USE_SSL)
+           {
+             gnutls_priority_deinit (daemon->priority_cache);
+             ret = gnutls_priority_init (&daemon->priority_cache,
+                                         pstr = va_arg (ap, const char*),
+                                         NULL);
 #if HAVE_MESSAGES
-         if (ret != GNUTLS_E_SUCCESS)
-           FPRINTF (stderr,
-                    "Setting priorities to `%s' failed: %s\n",
-                    pstr,
-                    gnutls_strerror (ret));
+             if (ret != GNUTLS_E_SUCCESS)
+               FPRINTF (stderr,
+                        "Setting priorities to `%s' failed: %s\n",
+                        pstr,
+                        gnutls_strerror (ret));
 #endif   
-         if (ret != GNUTLS_E_SUCCESS)
-           return MHD_NO;
+             if (ret != GNUTLS_E_SUCCESS)
+               return MHD_NO;
+           }
           break;
 #endif
 #ifdef DAUTH_SUPPORT
@@ -2174,6 +2196,16 @@
       CLOSE (socket_fd);
       goto free_and_fail;
     }
+  if (0 != pthread_mutex_init (&retVal->cleanup_connection_mutex, NULL))
+    {
+#if HAVE_MESSAGES
+      MHD_DLOG (retVal,
+               "MHD failed to initialize IP connection limit mutex\n");
+#endif
+      pthread_mutex_destroy (&retVal->cleanup_connection_mutex);
+      CLOSE (socket_fd);
+      goto free_and_fail;
+    }
 
 #if HTTPS_SUPPORT
   /* initialize HTTPS daemon certificate aspects & send / recv functions */
@@ -2184,6 +2216,7 @@
                "Failed to initialize TLS support\n");
 #endif
       CLOSE (socket_fd);
+      pthread_mutex_destroy (&retVal->cleanup_connection_mutex);
       pthread_mutex_destroy (&retVal->per_ip_connection_mutex);
       goto free_and_fail;
     }
@@ -2199,6 +2232,7 @@
                 "Failed to create listen thread: %s\n", 
                STRERROR (res_thread_create));
 #endif
+      pthread_mutex_destroy (&retVal->cleanup_connection_mutex);
       pthread_mutex_destroy (&retVal->per_ip_connection_mutex);
       CLOSE (socket_fd);
       goto free_and_fail;
@@ -2292,6 +2326,7 @@
   if (i == 0)
     {
       CLOSE (socket_fd);
+      pthread_mutex_destroy (&retVal->cleanup_connection_mutex);
       pthread_mutex_destroy (&retVal->per_ip_connection_mutex);
       if (NULL != retVal->worker_pool)
         free (retVal->worker_pool);
@@ -2319,27 +2354,45 @@
 
 
 /**
- * Close all connections for the daemon
+ * Close all connections for the daemon; must only be called after
+ * all of the threads have been joined and there is no more concurrent
+ * activity on the connection lists.
  *
  * @param daemon daemon to close down
  */
 static void
-MHD_close_connections (struct MHD_Daemon *daemon)
+close_all_connections (struct MHD_Daemon *daemon)
 {
-  while (daemon->connections != NULL)
+  struct MHD_Connection *pos;
+  void *unused;
+  int rc;
+
+  if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
     {
-      if (-1 != daemon->connections->socket_fd)
-        {
-#if DEBUG_CLOSE
+      while (NULL != (pos = daemon->connections_head))
+       {
+         if (0 != (rc = pthread_join (pos->pid, &unused)))
+           {
 #if HAVE_MESSAGES
-          MHD_DLOG (daemon, "MHD shutdown, closing active connections\n");
+             MHD_DLOG (daemon, "Failed to join a thread: %s\n",
+                       STRERROR (rc));
 #endif
-#endif
-          MHD_connection_close (daemon->connections,
-                                MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
-        }
-      MHD_cleanup_connections (daemon);
+             abort();
+           }
+         pos->thread_joined = MHD_YES;
+       }
     }
+  while (NULL != (pos = daemon->connections_head))
+    {
+      pos->state = MHD_CONNECTION_CLOSED;
+      DLL_remove (daemon->connections_head,
+                 daemon->connections_tail,
+                 pos);
+      DLL_insert (daemon->cleanup_head,
+                 daemon->cleanup_tail,
+                 pos);
+    }
+  MHD_cleanup_connections (daemon);
 }
 
 
@@ -2387,7 +2440,7 @@
 #endif
          abort();
        }
-      MHD_close_connections (&daemon->worker_pool[i]);
+      close_all_connections (&daemon->worker_pool[i]);
     }
   free (daemon->worker_pool);
 
@@ -2404,7 +2457,7 @@
          abort();
        }
     }
-  MHD_close_connections (daemon);
+  close_all_connections (daemon);
   CLOSE (fd);
 
   /* TLS clean up */
@@ -2437,7 +2490,7 @@
   pthread_mutex_destroy (&daemon->nnc_lock);
 #endif
   pthread_mutex_destroy (&daemon->per_ip_connection_mutex);
-
+  pthread_mutex_destroy (&daemon->cleanup_connection_mutex);
   free (daemon);
 }
 

Modified: libmicrohttpd/src/daemon/internal.h
===================================================================
--- libmicrohttpd/src/daemon/internal.h 2011-06-21 12:46:43 UTC (rev 15730)
+++ libmicrohttpd/src/daemon/internal.h 2011-06-21 13:00:30 UTC (rev 15731)
@@ -379,11 +379,15 @@
   MHD_CONNECTION_FOOTERS_SENT = MHD_CONNECTION_FOOTERS_SENDING + 1,
 
   /**
-   * 19: This connection is closed (no more activity
-   * allowed).
+   * 19: This connection is to be closed.
    */
   MHD_CONNECTION_CLOSED = MHD_CONNECTION_FOOTERS_SENT + 1,
 
+  /**
+   * 20: This connection is finished (only to be freed)
+   */
+  MHD_CONNECTION_IN_CLEANUP = MHD_CONNECTION_CLOSED + 1,
+
   /*
    *  SSL/TLS connection states
    */
@@ -440,11 +444,16 @@
 {
 
   /**
-   * This is a linked list.
+   * This is a doubly-linked list.
    */
   struct MHD_Connection *next;
 
   /**
+   * This is a doubly-linked list.
+   */
+  struct MHD_Connection *prev;
+
+  /**
    * Reference to the MHD_Daemon struct.
    */
   struct MHD_Daemon *daemon;
@@ -622,6 +631,11 @@
   int read_closed;
 
   /**
+   * Set to MHD_YES if the thread has been joined.
+   */
+  int thread_joined;
+
+  /**
    * State in the FSM for this connection.
    */
   enum MHD_CONNECTION_STATE state;
@@ -747,11 +761,26 @@
   void *default_handler_cls;
 
   /**
-   * Linked list of our current connections.
+   * Tail of doubly-linked list of our current, active connections.
    */
-  struct MHD_Connection *connections;
+  struct MHD_Connection *connections_head;
 
   /**
+   * Tail of doubly-linked list of our current, active connections.
+   */
+  struct MHD_Connection *connections_tail;
+
+  /**
+   * Tail of doubly-linked list of connections to clean up.
+   */
+  struct MHD_Connection *cleanup_head;
+
+  /**
+   * Tail of doubly-linked list of connections to clean up.
+   */
+  struct MHD_Connection *cleanup_tail;
+
+  /**
    * Function to call to check if we should
    * accept or reject an incoming request.
    * May be NULL.
@@ -847,11 +876,16 @@
   pthread_t pid;
 
   /**
-   * Mutex for per-IP connection counts
+   * Mutex for per-IP connection counts.
    */
   pthread_mutex_t per_ip_connection_mutex;
 
   /**
+   * Mutex for (modifying) access to the "cleanup" connection DLL.
+   */
+  pthread_mutex_t cleanup_connection_mutex;
+
+  /**
    * Listen socket.
    */
   int socket_fd;
@@ -966,5 +1000,44 @@
 #endif
 
 
+/**
+ * Insert an element at the head of a DLL. Assumes that head, tail and
+ * element are structs with prev and next fields.
+ *
+ * @param head pointer to the head of the DLL
+ * @param tail pointer to the tail of the DLL
+ * @param element element to insert
+ */
+#define DLL_insert(head,tail,element) do { \
+  (element)->next = (head); \
+  (element)->prev = NULL; \
+  if ((tail) == NULL) \
+    (tail) = element; \
+  else \
+    (head)->prev = element; \
+  (head) = (element); } while (0)
 
+
+/**
+ * Remove an element from a DLL. Assumes
+ * that head, tail and element are structs
+ * with prev and next fields.
+ *
+ * @param head pointer to the head of the DLL
+ * @param tail pointer to the tail of the DLL
+ * @param element element to remove
+ */
+#define DLL_remove(head,tail,element) do { \
+  if ((element)->prev == NULL) \
+    (head) = (element)->next;  \
+  else \
+    (element)->prev->next = (element)->next; \
+  if ((element)->next == NULL) \
+    (tail) = (element)->prev;  \
+  else \
+    (element)->next->prev = (element)->prev; \
+  (element)->next = NULL; \
+  (element)->prev = NULL; } while (0)
+
+
 #endif




reply via email to

[Prev in Thread] Current Thread [Next in Thread]