commit 2a6eada873b10f20c9a7366d19bad98b9bd945df
parent 7131394fa881b732cd141fda9875953f5b5c1831
Author: Christian Grothoff <christian@grothoff.org>
Date: Sun, 25 Oct 2015 14:33:28 +0000
-fix assertion failure from race on shutdown and buffer shrinkage with pipelining
Diffstat:
7 files changed, 108 insertions(+), 57 deletions(-)
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,12 @@
+Sun Oct 25 15:29:23 CET 2015
+ Fixing transient resource leak affecting long-lived
+ connections with many keep-alives and HTTP request
+ pipelining under certain circumstances (which reduced
+ the receive window).
+ Fixed assertion failure triggered by a race in
+ thread-per-connection mode on shutdown in rare
+ circumstances. -CG
+
Mon Oct 5 11:53:52 CEST 2015
Deduplicate code between digestauth and connection
parsing logic for URI arguments, shared code moved
diff --git a/src/include/microhttpd.h b/src/include/microhttpd.h
@@ -130,7 +130,7 @@ typedef intptr_t ssize_t;
* Current version of the library.
* 0x01093001 = 1.9.30-1.
*/
-#define MHD_VERSION 0x00094401
+#define MHD_VERSION 0x00094402
/**
* MHD-internal return code for "YES".
diff --git a/src/microhttpd/connection.c b/src/microhttpd/connection.c
@@ -264,8 +264,8 @@ need_100_continue (struct MHD_Connection *connection)
* @param termination_code termination reason to give
*/
void
-MHD_connection_close (struct MHD_Connection *connection,
- enum MHD_RequestTerminationCode termination_code)
+MHD_connection_close_ (struct MHD_Connection *connection,
+ enum MHD_RequestTerminationCode termination_code)
{
struct MHD_Daemon *daemon;
@@ -300,8 +300,8 @@ connection_close_error (struct MHD_Connection *connection,
if (NULL != emsg)
MHD_DLOG (connection->daemon, emsg);
#endif
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_WITH_ERROR);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_WITH_ERROR);
}
@@ -368,8 +368,8 @@ try_ready_normal_body (struct MHD_Connection *connection)
if (NULL != response->crc)
(void) MHD_mutex_unlock_ (&response->mutex);
if ( ((ssize_t)MHD_CONTENT_READER_END_OF_STREAM) == ret)
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_COMPLETED_OK);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_COMPLETED_OK);
else
CONNECTION_CLOSE_ERROR (connection,
"Closing connection (stream error)\n");
@@ -1611,8 +1611,8 @@ do_read (struct MHD_Connection *connection)
/* other side closed connection; RFC 2616, section 8.1.4 suggests
we should then shutdown ourselves as well. */
connection->read_closed = MHD_YES;
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_CLIENT_ABORT);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_CLIENT_ABORT);
return MHD_YES;
}
connection->read_buffer_offset += bytes_read;
@@ -1959,8 +1959,8 @@ MHD_connection_handle_read (struct MHD_Connection *connection)
/* nothing to do but default action */
if (MHD_YES == connection->read_closed)
{
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_READ_ERROR);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_READ_ERROR);
continue;
}
break;
@@ -2539,8 +2539,8 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
(MHD_NO == keepalive_possible (connection)))
{
/* have to close for some reason */
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_COMPLETED_OK);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_COMPLETED_OK);
MHD_pool_destroy (connection->pool);
connection->pool = NULL;
connection->read_buffer = NULL;
@@ -2555,7 +2555,7 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
connection->read_buffer
= MHD_pool_reset (connection->pool,
connection->read_buffer,
- connection->read_buffer_size);
+ connection->read_buffer_offset);
}
connection->client_aware = MHD_NO;
connection->client_context = NULL;
@@ -2585,8 +2585,8 @@ MHD_connection_handle_idle (struct MHD_Connection *connection)
if ( (0 != timeout) &&
(timeout <= (MHD_monotonic_sec_counter() - connection->last_activity)) )
{
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
connection->in_idle = MHD_NO;
return MHD_YES;
}
diff --git a/src/microhttpd/connection.h b/src/microhttpd/connection.h
@@ -89,8 +89,8 @@ MHD_connection_handle_idle (struct MHD_Connection *connection);
* @param termination_code termination reason to give
*/
void
-MHD_connection_close (struct MHD_Connection *connection,
- enum MHD_RequestTerminationCode termination_code);
+MHD_connection_close_ (struct MHD_Connection *connection,
+ enum MHD_RequestTerminationCode termination_code);
#if EPOLL_SUPPORT
diff --git a/src/microhttpd/connection_https.c b/src/microhttpd/connection_https.c
@@ -69,8 +69,8 @@ run_tls_handshake (struct MHD_Connection *connection)
MHD_DLOG (connection->daemon,
"Error: received handshake message out of context\n");
#endif
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_WITH_ERROR);
+ MHD_connection_close_ (connection,
+ MHD_REQUEST_TERMINATED_WITH_ERROR);
return MHD_YES;
}
return MHD_NO;
@@ -142,8 +142,8 @@ MHD_tls_connection_handle_idle (struct MHD_Connection *connection)
#endif
timeout = connection->connection_timeout;
if ( (timeout != 0) && (timeout <= (MHD_monotonic_sec_counter() - connection->last_activity)))
- MHD_connection_close (connection,
- MHD_REQUEST_TERMINATED_TIMEOUT_REACHED);
+ 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 */
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
@@ -135,7 +135,8 @@ mhd_panic_std (void *cls,
const char *reason)
{
#if HAVE_MESSAGES
- fprintf (stderr, "Fatal error in GNU libmicrohttpd %s:%u: %s\n",
+ fprintf (stderr,
+ "Fatal error in GNU libmicrohttpd %s:%u: %s\n",
file, line, reason);
#endif
abort ();
@@ -994,7 +995,8 @@ MHD_handle_connection (void *data)
if (0 != (p[0].revents & POLLOUT))
con->write_handler (con);
if (0 != (p[0].revents & (POLLERR | POLLHUP)))
- MHD_connection_close (con, MHD_REQUEST_TERMINATED_WITH_ERROR);
+ MHD_connection_close_ (con,
+ MHD_REQUEST_TERMINATED_WITH_ERROR);
if (MHD_NO == con->idle_handler (con))
goto exit;
}
@@ -1009,8 +1011,8 @@ MHD_handle_connection (void *data)
#endif
#endif
if (MHD_CONNECTION_CLOSED != con->state)
- MHD_connection_close (con,
- MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
+ MHD_connection_close_ (con,
+ MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
con->idle_handler (con);
}
exit:
@@ -4359,8 +4361,8 @@ close_connection (struct MHD_Connection *pos)
{
struct MHD_Daemon *daemon = pos->daemon;
- MHD_connection_close (pos,
- MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
+ MHD_connection_close_ (pos,
+ MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN);
if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
return; /* must let thread to the rest */
if (pos->connection_timeout == pos->daemon->connection_timeout)
@@ -4398,35 +4400,50 @@ close_all_connections (struct MHD_Daemon *daemon)
if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
(MHD_YES != MHD_mutex_lock_ (&daemon->cleanup_connection_mutex)) )
MHD_PANIC ("Failed to acquire cleanup mutex\n");
+ if (NULL != daemon->suspended_connections_head)
+ MHD_PANIC ("MHD_stop_daemon() called while we have suspended connections.\n");
for (pos = daemon->connections_head; NULL != pos; pos = pos->next)
{
shutdown (pos->socket_fd,
- (pos->read_closed == MHD_YES) ? SHUT_WR : SHUT_RDWR);
+ (MHD_YES == pos->read_closed) ? SHUT_WR : SHUT_RDWR);
#if MHD_WINSOCK_SOCKETS
if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
(MHD_INVALID_PIPE_ != daemon->wpipe[1]) &&
(1 != MHD_pipe_write_ (daemon->wpipe[1], "e", 1)) )
- MHD_PANIC ("failed to signal shutdown via pipe");
+ MHD_PANIC ("Failed to signal shutdown via pipe");
#endif
}
if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
(MHD_YES != MHD_mutex_unlock_ (&daemon->cleanup_connection_mutex)) )
MHD_PANIC ("Failed to release cleanup mutex\n");
- /* now, collect threads from thread pool */
+ /* now, collect per-connection threads */
if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
{
- for (pos = daemon->connections_head; NULL != pos; pos = pos->next)
- {
- if (0 != MHD_join_thread_ (pos->pid))
- MHD_PANIC ("Failed to join a thread\n");
- pos->thread_joined = MHD_YES;
- }
+ pos = daemon->connections_head;
+ while (NULL != pos)
+ {
+ if (MHD_YES != pos->thread_joined)
+ {
+ if (0 != MHD_join_thread_ (pos->pid))
+ MHD_PANIC ("Failed to join a thread\n");
+ pos->thread_joined = MHD_YES;
+ /* The thread may have concurrently modified the DLL,
+ need to restart from the beginning */
+ pos = daemon->connections_head;
+ continue;
+ }
+ pos = pos->next;
+ }
}
-
/* now that we're alone, move everyone to cleanup */
while (NULL != (pos = daemon->connections_head))
+ {
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (MHD_YES != pos->thread_joined) )
+ MHD_PANIC ("Failed to join a thread\n");
close_connection (pos);
+ }
MHD_cleanup_connections (daemon);
}
@@ -4478,7 +4495,7 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
/* Prepare workers for shutdown */
if (NULL != daemon->worker_pool)
{
- /* MHD_USE_NO_LISTEN_SOCKET disables thread pools, hence we need to check */
+ /* #MHD_USE_NO_LISTEN_SOCKET disables thread pools, hence we need to check */
for (i = 0; i < daemon->worker_pool_size; ++i)
{
daemon->worker_pool[i].shutdown = MHD_YES;
@@ -4556,9 +4573,9 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
else
{
/* clean up master threads */
- if ((0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) ||
- ((0 != (daemon->options & MHD_USE_SELECT_INTERNALLY))
- && (0 == daemon->worker_pool_size)))
+ if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) ||
+ ( (0 != (daemon->options & MHD_USE_SELECT_INTERNALLY)) &&
+ (0 == daemon->worker_pool_size) ) )
{
if (0 != MHD_join_thread_ (daemon->pid))
{
@@ -4676,7 +4693,8 @@ MHD_get_daemon_info (struct MHD_Daemon *daemon,
* @ingroup logging
*/
void
-MHD_set_panic_func (MHD_PanicCallback cb, void *cls)
+MHD_set_panic_func (MHD_PanicCallback cb,
+ void *cls)
{
mhd_panic = cb;
mhd_panic_cls = cls;
@@ -4825,13 +4843,14 @@ MHD_is_feature_supported(enum MHD_FEATURE feature)
#if defined(MHD_USE_POSIX_THREADS)
GCRY_THREAD_OPTION_PTHREAD_IMPL;
#elif defined(MHD_W32_MUTEX_)
-static int gcry_w32_mutex_init (void **ppmtx)
+
+static int
+gcry_w32_mutex_init (void **ppmtx)
{
*ppmtx = malloc (sizeof (MHD_mutex_));
if (NULL == *ppmtx)
return ENOMEM;
-
if (MHD_YES != MHD_mutex_create_ ((MHD_mutex_*)*ppmtx))
{
free (*ppmtx);
@@ -4841,13 +4860,30 @@ static int gcry_w32_mutex_init (void **ppmtx)
return 0;
}
-static int gcry_w32_mutex_destroy (void **ppmtx)
- { int res = (MHD_YES == MHD_mutex_destroy_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1;
- free (*ppmtx); return res; }
-static int gcry_w32_mutex_lock (void **ppmtx)
- { return (MHD_YES == MHD_mutex_lock_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1; }
-static int gcry_w32_mutex_unlock (void **ppmtx)
- { return (MHD_YES == MHD_mutex_unlock_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1; }
+
+
+static int
+gcry_w32_mutex_destroy (void **ppmtx)
+{
+ int res = (MHD_YES == MHD_mutex_destroy_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1;
+ free (*ppmtx);
+ return res;
+}
+
+
+static int
+gcry_w32_mutex_lock (void **ppmtx)
+{
+ return (MHD_YES == MHD_mutex_lock_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1;
+}
+
+
+static int
+gcry_w32_mutex_unlock (void **ppmtx)
+{
+ return (MHD_YES == MHD_mutex_unlock_ ((MHD_mutex_*)*ppmtx)) ? 0 : 1;
+}
+
static struct gcry_thread_cbs gcry_threads_w32 = {
(GCRY_THREAD_OPTION_USER | (GCRY_THREAD_OPTION_VERSION << 8)),
@@ -4862,7 +4898,8 @@ static struct gcry_thread_cbs gcry_threads_w32 = {
/**
* Initialize do setup work.
*/
-void MHD_init(void)
+void
+MHD_init(void)
{
mhd_panic = &mhd_panic_std;
mhd_panic_cls = NULL;
@@ -4895,7 +4932,8 @@ void MHD_init(void)
}
-void MHD_fini(void)
+void
+MHD_fini(void)
{
#if HTTPS_SUPPORT
gnutls_global_deinit ();
diff --git a/src/testcurl/test_concurrent_stop.c b/src/testcurl/test_concurrent_stop.c
@@ -68,13 +68,15 @@ copyBuffer (void *ptr,
return size * nmemb;
}
+
static int
ahc_echo (void *cls,
struct MHD_Connection *connection,
const char *url,
const char *method,
const char *version,
- const char *upload_data, size_t *upload_data_size,
+ const char *upload_data,
+ size_t *upload_data_size,
void **unused)
{
static int ptr;
@@ -166,7 +168,8 @@ join_gets (pid_t pid)
static int
-testMultithreadedGet (int port, int poll_flag)
+testMultithreadedGet (int port,
+ int poll_flag)
{
struct MHD_Daemon *d;
pid_t p;
@@ -184,7 +187,8 @@ testMultithreadedGet (int port, int poll_flag)
static int
-testMultithreadedPoolGet (int port, int poll_flag)
+testMultithreadedPoolGet (int port,
+ int poll_flag)
{
struct MHD_Daemon *d;
pid_t p;
