commit 901090b07ba9c89e086072b14768a24241f6714c
parent 94bb1bb839e1dbb9e84f0b3a31cc8689df9a9d54
Author: Christian Grothoff <christian@grothoff.org>
Date: Tue, 21 Jun 2011 13:00:30 +0000
fixing race condition, minor leak, O(n) shutdown is now O(1)
Diffstat:
5 files changed, 387 insertions(+), 274 deletions(-)
diff --git a/ChangeLog b/ChangeLog
@@ -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
diff --git a/src/daemon/connection.c b/src/daemon/connection.c
@@ -275,6 +275,7 @@ need_100_continue (struct MHD_Connection *connection)
strlen (HTTP_100_CONTINUE)));
}
+
/**
* Close the given connection and give the
* specified termination code to the user.
@@ -283,31 +284,52 @@ void
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 @@ try_ready_normal_body (struct MHD_Connection *connection)
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 @@ try_ready_chunked_body (struct MHD_Connection *connection)
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 @@ try_ready_chunked_body (struct MHD_Connection *connection)
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 @@ transmit_error_response (struct MHD_Connection *connection,
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 @@ MHD_connection_get_pollfd (struct MHD_Connection *connection, struct MHD_Pollfd
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 @@ MHD_connection_get_pollfd (struct MHD_Connection *connection, struct MHD_Pollfd
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 @@ MHD_connection_get_pollfd (struct MHD_Connection *connection, struct MHD_Pollfd
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 @@ MHD_connection_get_pollfd (struct MHD_Connection *connection, struct MHD_Pollfd
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 @@ call_connection_handler (struct MHD_Connection *connection)
&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 @@ process_request_body (struct MHD_Connection *connection)
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 @@ process_request_body (struct MHD_Connection *connection)
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 @@ process_request_body (struct MHD_Connection *connection)
&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 @@ do_read (struct MHD_Connection *connection)
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 @@ do_write (struct MHD_Connection *connection)
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 @@ process_header_line (struct MHD_Connection *connection, char *line)
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 @@ parse_connection_headers (struct MHD_Connection *connection)
"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 @@ parse_connection_headers (struct MHD_Connection *connection)
* 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 @@ MHD_connection_handle_read (struct MHD_Connection *connection)
}
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 @@ MHD_connection_handle_read (struct MHD_Connection *connection)
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_connection_handle_write (struct MHD_Connection *connection)
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_connection_handle_write (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_write (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_write (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
(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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
}
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 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
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;
}
diff --git a/src/daemon/connection_https.c b/src/daemon/connection_https.c
@@ -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 @@ MHD_tls_connection_close (struct MHD_Connection *connection,
* 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_tls_connection_handle_read (struct MHD_Connection *connection)
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 @@ MHD_tls_connection_handle_read (struct MHD_Connection *connection)
* 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_tls_connection_handle_write (struct MHD_Connection *connection)
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 @@ MHD_tls_connection_handle_idle (struct MHD_Connection *connection)
__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 @@ MHD_tls_connection_handle_idle (struct MHD_Connection *connection)
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;
diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
@@ -150,7 +150,7 @@ struct MHD_IPCount
};
/**
- * 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 @@ MHD_ip_count_lock(struct MHD_Daemon *daemon)
}
/**
- * 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 @@ MHD_TLS_init (struct MHD_Daemon *daemon)
}
#endif
+
/**
* Obtain the select sets for this daemon.
*
@@ -513,7 +514,8 @@ MHD_get_fdset (struct MHD_Daemon *daemon,
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 @@ MHD_get_fdset (struct MHD_Daemon *daemon,
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 @@ MHD_handle_connection (void *data)
#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 @@ MHD_handle_connection (void *data)
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 @@ MHD_handle_connection (void *data)
#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 @@ MHD_handle_connection (void *data)
"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 @@ recv_param_adapter (struct MHD_Connection *connection,
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 @@ send_param_adapter (struct MHD_Connection *connection,
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 @@ MHD_add_connection (struct MHD_Daemon *daemon,
/* 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 @@ MHD_add_connection (struct MHD_Daemon *daemon,
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 @@ MHD_accept_connection (struct MHD_Daemon *daemon)
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 @@ static void
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));
-#endif
- abort();
- }
- }
- MHD_pool_destroy (pos->pool);
-#if HTTPS_SUPPORT
- if (pos->tls_session != NULL)
- gnutls_deinit (pos->tls_session);
+ MHD_DLOG (daemon, "Failed to acquire cleanup mutex\n");
#endif
- MHD_ip_limit_del (daemon, (struct sockaddr*)pos->addr, pos->addr_len);
- if (pos->response != NULL)
+ 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)))
{
- MHD_destroy_response (pos->response);
- pos->response = NULL;
+#if HAVE_MESSAGES
+ MHD_DLOG (daemon, "Failed to join a thread: %s\n",
+ STRERROR (rc));
+#endif
+ abort();
}
- 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_pool_destroy (pos->pool);
+#if HTTPS_SUPPORT
+ 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;
+ }
+ 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 @@ MHD_get_timeout (struct MHD_Daemon *daemon,
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 @@ MHD_select (struct MHD_Daemon *daemon,
int may_block)
{
struct MHD_Connection *pos;
+ struct MHD_Connection *next;
int num_ready;
fd_set rs;
fd_set ws;
@@ -1313,21 +1333,19 @@ MHD_select (struct MHD_Daemon *daemon,
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 @@ MHD_poll_all (struct MHD_Daemon *daemon,
{
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 @@ MHD_poll_all (struct MHD_Daemon *daemon,
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 @@ MHD_poll_all (struct MHD_Daemon *daemon,
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 @@ MHD_poll_all (struct MHD_Daemon *daemon,
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 @@ parse_options_va (struct MHD_Daemon *daemon,
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 @@ MHD_start_daemon_va (unsigned int options,
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 @@ MHD_start_daemon_va (unsigned int options,
"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 @@ MHD_start_daemon_va (unsigned int options,
"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 @@ thread_failed:
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 @@ thread_failed:
/**
- * 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 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
#endif
abort();
}
- MHD_close_connections (&daemon->worker_pool[i]);
+ close_all_connections (&daemon->worker_pool[i]);
}
free (daemon->worker_pool);
@@ -2404,7 +2457,7 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
abort();
}
}
- MHD_close_connections (daemon);
+ close_all_connections (daemon);
CLOSE (fd);
/* TLS clean up */
@@ -2437,7 +2490,7 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
pthread_mutex_destroy (&daemon->nnc_lock);
#endif
pthread_mutex_destroy (&daemon->per_ip_connection_mutex);
-
+ pthread_mutex_destroy (&daemon->cleanup_connection_mutex);
free (daemon);
}
diff --git a/src/daemon/internal.h b/src/daemon/internal.h
@@ -379,11 +379,15 @@ enum MHD_CONNECTION_STATE
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 @@ struct MHD_Connection
{
/**
- * 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 @@ struct MHD_Connection
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,9 +761,24 @@ struct MHD_Daemon
void *default_handler_cls;
/**
- * Linked list of our current connections.
+ * Tail of doubly-linked list of our current, active 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 *connections;
+ 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
@@ -847,11 +876,16 @@ struct MHD_Daemon
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 @@ struct MHD_Daemon
#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
