commit 0ebdff94d461abd4328cf45a6281c15139a045eb
parent 73532f78bb3a1a07c091ef7123b3ab49347c0b95
Author: Christian Grothoff <christian@grothoff.org>
Date: Fri, 2 Mar 2018 21:45:17 +0100
fix test_upgrade transient failures (#5189)
Diffstat:
6 files changed, 129 insertions(+), 66 deletions(-)
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,7 @@
+Fri Mar 2 21:44:24 CET 2018
+ Ensure MHD_RequestCompletedCallback is always called from
+ the correct thread (even on shutdown and for upgraded connections). -CG
+
Tue Feb 27 23:27:02 CET 2018
Ensure MHD_RequestCompletedCallback is also called for
upgraded connections. -CG
diff --git a/src/lib/daemon_destroy.c b/src/lib/daemon_destroy.c
@@ -37,7 +37,7 @@ stop_workers (struct MHD_Daemon *daemon)
{
MHD_socket fd;
unsigned int i;
-
+
mhd_assert (1 < daemon->worker_pool_size);
mhd_assert (1 < daemon->threading_model);
if (daemon->was_quiesced)
@@ -102,7 +102,7 @@ MHD_daemon_destroy (struct MHD_Daemon *daemon)
fd = daemon->listen_socket;
/* FIXME: convert from here to microhttpd2-style API! */
-
+
if (NULL != daemon->worker_pool)
{ /* Master daemon with worker pool. */
stop_workers (daemon);
@@ -114,9 +114,6 @@ MHD_daemon_destroy (struct MHD_Daemon *daemon)
if (MHD_TM_EXTERNAL_EVENT_LOOP != daemon->threading_model)
{
/* Worker daemon or single daemon with internal thread(s). */
- if (! daemon->disallow_suspend_resume)
- (void) MHD_resume_suspended_connections_ (daemon);
-
/* Separate thread(s) is used for polling sockets. */
if (MHD_ITC_IS_VALID_(daemon->itc))
{
@@ -137,7 +134,7 @@ MHD_daemon_destroy (struct MHD_Daemon *daemon)
#endif /* HAVE_LISTEN_SHUTDOWN */
mhd_assert (false); /* Should never happen */
}
-
+
if (! MHD_join_thread_ (daemon->pid.handle))
{
MHD_PANIC (_("Failed to join a thread\n"));
@@ -171,10 +168,10 @@ MHD_daemon_destroy (struct MHD_Daemon *daemon)
return;
/* Cleanup that should be done only one time in master/single daemon.
* Do not perform this cleanup in worker daemons. */
-
+
if (MHD_INVALID_SOCKET != fd)
MHD_socket_close_chk_ (fd);
-
+
/* TLS clean up */
#ifdef HTTPS_SUPPORT
if (NULL != daemon->tls_api)
@@ -191,7 +188,7 @@ MHD_daemon_destroy (struct MHD_Daemon *daemon)
#endif
}
#endif /* HTTPS_SUPPORT */
-
+
#ifdef DAUTH_SUPPORT
free (daemon->nnc);
MHD_mutex_destroy_chk_ (&daemon->nnc_lock);
diff --git a/src/lib/daemon_start.c b/src/lib/daemon_start.c
@@ -28,6 +28,7 @@
#include "daemon_select.h"
#include "daemon_poll.h"
#include "daemon_epoll.h"
+#include "request_resume.h"
/**
@@ -159,7 +160,7 @@ open_listen_socket (struct MHD_Daemon *daemon)
const struct sockaddr *sa;
int pf;
bool use_v6;
-
+
if (MHD_INVALID_SOCKET != daemon->listen_socket)
return MHD_SC_OK; /* application opened it for us! */
@@ -223,7 +224,7 @@ open_listen_socket (struct MHD_Daemon *daemon)
return MHD_SC_IPV6_NOT_SUPPORTED_BY_BUILD;
#endif
}
-
+
/* try to open listen socket */
try_open_listen_socket:
daemon->listen_socket = MHD_socket_create_listen_(pf);
@@ -235,7 +236,7 @@ open_listen_socket (struct MHD_Daemon *daemon)
pf = PF_INET;
goto try_open_listen_socket;
}
- if (MHD_INVALID_SOCKET == daemon->listen_socket)
+ if (MHD_INVALID_SOCKET == daemon->listen_socket)
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
@@ -281,7 +282,7 @@ open_listen_socket (struct MHD_Daemon *daemon)
#endif
#endif
}
-
+
/* Determine address to bind to */
if (0 != daemon->listen_sa_len)
{
@@ -332,8 +333,8 @@ open_listen_socket (struct MHD_Daemon *daemon)
}
sa = (const struct sockaddr *) &ss;
}
-
- /* actually do the bind() */
+
+ /* actually do the bind() */
if (-1 == bind (daemon->listen_socket,
sa,
addrlen))
@@ -406,7 +407,7 @@ open_listen_socket (struct MHD_Daemon *daemon)
}
-/**
+/**
* Obtain the listen port number from the socket (if it
* was not explicitly set by us, i.e. if we were given
* a listen socket or if the port was 0 and the OS picked
@@ -419,7 +420,7 @@ get_listen_port_number (struct MHD_Daemon *daemon)
{
struct sockaddr_storage servaddr;
socklen_t addrlen;
-
+
if ( (0 != daemon->listen_port) ||
(MHD_INVALID_SOCKET == daemon->listen_socket) )
return; /* nothing to be done */
@@ -457,7 +458,7 @@ get_listen_port_number (struct MHD_Daemon *daemon)
case AF_INET:
{
struct sockaddr_in *s4 = (struct sockaddr_in *) &servaddr;
-
+
daemon->listen_port = ntohs (s4->sin_port);
break;
}
@@ -465,7 +466,7 @@ get_listen_port_number (struct MHD_Daemon *daemon)
case AF_INET6:
{
struct sockaddr_in6 *s6 = (struct sockaddr_in6 *) &servaddr;
-
+
daemon->listen_port = ntohs(s6->sin6_port);
break;
}
@@ -630,7 +631,7 @@ MHD_polling_thread (void *cls)
MHD_YES);
break;
case MHD_ELS_EPOLL:
-#ifdef EPOLL_SUPPORT
+#ifdef EPOLL_SUPPORT
MHD_daemon_epoll_ (daemon,
MHD_YES);
#else
@@ -643,6 +644,8 @@ MHD_polling_thread (void *cls)
/* Resume any pending for resume connections, join
* all connection's threads (if any) and finally cleanup
* everything. */
+ if (! daemon->disallow_suspend_resume)
+ MHD_resume_suspended_connections_ (daemon);
MHD_daemon_close_all_connections_ (daemon);
return (MHD_THRD_RTRN_TYPE_)0;
@@ -673,13 +676,13 @@ setup_thread_pool (struct MHD_Daemon *daemon)
sizeof (struct MHD_Daemon));
if (NULL == daemon->worker_pool)
return MHD_SC_THREAD_POOL_MALLOC_FAILURE;
-
+
/* Start the workers in the pool */
for (i = 0; i < daemon->threading_model; i++)
{
/* Create copy of the Daemon object for each worker */
struct MHD_Daemon *d = &daemon->worker_pool[i];
-
+
memcpy (d,
daemon,
sizeof (struct MHD_Daemon));
@@ -695,8 +698,8 @@ setup_thread_pool (struct MHD_Daemon *daemon)
d->global_connection_limit = conns_per_thread;
if (((unsigned int) i) < leftover_conns)
++d->global_connection_limit;
-
- if (! daemon->disable_itc)
+
+ if (! daemon->disable_itc)
{
if (! MHD_itc_init_ (d->itc))
{
@@ -727,13 +730,13 @@ setup_thread_pool (struct MHD_Daemon *daemon)
{
MHD_itc_set_invalid_ (d->itc);
}
-
+
#ifdef EPOLL_SUPPORT
if ( (MHD_ELS_EPOLL == daemon->event_loop_syscall) &&
(MHD_SC_OK != (sc = setup_epoll_to_listen (d))) )
goto thread_failed;
#endif
-
+
/* Must init cleanup connection mutex for each worker */
if (! MHD_mutex_init_ (&d->cleanup_connection_mutex))
{
@@ -747,7 +750,7 @@ setup_thread_pool (struct MHD_Daemon *daemon)
sc = MHD_SC_THREAD_POOL_CREATE_MUTEX_FAILURE;
goto thread_failed;
}
-
+
/* Spawn the worker thread */
if (! MHD_create_named_thread_ (&d->pid,
"MHD-worker",
@@ -867,10 +870,10 @@ MHD_daemon_start (struct MHD_Daemon *daemon)
return MHD_SC_ITC_DESCRIPTOR_TOO_LARGE;
}
}
-
+
if (MHD_SC_OK != (sc = open_listen_socket (daemon)))
return sc;
-
+
/* Check listen socket is in range (if we are limited) */
if ( (MHD_INVALID_SOCKET != daemon->listen_socket) &&
(MHD_ELS_SELECT == daemon->event_loop_syscall) &&
@@ -886,7 +889,7 @@ MHD_daemon_start (struct MHD_Daemon *daemon)
#endif
return MHD_SC_LISTEN_SOCKET_TOO_LARGE;
}
-
+
/* set listen socket to non-blocking */
if ( (MHD_INVALID_SOCKET != daemon->listen_socket) &&
(! MHD_socket_nonblocking_ (daemon->listen_socket)) )
@@ -907,7 +910,7 @@ MHD_daemon_start (struct MHD_Daemon *daemon)
return MHD_SC_LISTEN_SOCKET_NONBLOCKING_FAILURE;
}
}
-
+
#ifdef EPOLL_SUPPORT
/* Setup epoll */
if ( (MHD_ELS_EPOLL == daemon->event_loop_syscall) &&
@@ -916,7 +919,7 @@ MHD_daemon_start (struct MHD_Daemon *daemon)
(MHD_SC_OK != (sc = setup_epoll_to_listen (daemon))) )
return sc;
#endif
-
+
/* Setup main listen thread (only if we have no thread pool or
external event loop and do have a listen socket) */
/* FIXME: why no worker thread if we have no listen socket? */
@@ -949,7 +952,7 @@ MHD_daemon_start (struct MHD_Daemon *daemon)
/* make sure we know our listen port (if any) */
get_listen_port_number (daemon);
-
+
return MHD_SC_OK;
}
diff --git a/src/lib/request_resume.c b/src/lib/request_resume.c
@@ -156,9 +156,17 @@ MHD_resume_suspended_connections_ (struct MHD_Daemon *daemon)
#ifdef UPGRADE_SUPPORT
else
{
+ struct MHD_Response *response = pos->request.response;
+
/* Data forwarding was finished (for TLS connections) AND
* application was closed upgraded connection.
* Insert connection into cleanup list. */
+ if ( (NULL != response) &&
+ (MHD_TM_THREAD_PER_CONNECTION != daemon->threading_model) &&
+ (NULL != response->termination_cb) )
+ response->termination_cb (response->termination_cb_cls,
+ MHD_REQUEST_TERMINATED_COMPLETED_OK,
+ &pos->request.client_context);
DLL_insert (daemon->cleanup_head,
daemon->cleanup_tail,
pos);
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
@@ -2750,7 +2750,7 @@ resume_suspended_connections (struct MHD_Daemon *daemon)
DLL_insert (daemon->connections_head,
daemon->connections_tail,
pos);
- if (!used_thr_p_c)
+ if (! used_thr_p_c)
{
/* Reset timeout timer on resume. */
if (0 != pos->connection_timeout)
@@ -2787,6 +2787,15 @@ resume_suspended_connections (struct MHD_Daemon *daemon)
/* Data forwarding was finished (for TLS connections) AND
* application was closed upgraded connection.
* Insert connection into cleanup list. */
+
+ if ( (NULL != daemon->notify_completed) &&
+ (0 == (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+ (pos->client_aware) )
+ daemon->notify_completed (daemon->notify_completed_cls,
+ pos,
+ &pos->client_context,
+ MHD_REQUEST_TERMINATED_COMPLETED_OK);
+ pos->client_aware = false;
DLL_insert (daemon->cleanup_head,
daemon->cleanup_tail,
pos);
@@ -3780,7 +3789,7 @@ MHD_poll_listen_socket (struct MHD_Daemon *daemon,
}
if (0 != (daemon->options & MHD_TEST_ALLOW_SUSPEND_RESUME))
- (void)resume_suspended_connections (daemon);
+ (void) resume_suspended_connections (daemon);
if (MHD_NO == may_block)
timeout = 0;
@@ -4385,7 +4394,7 @@ close_connection (struct MHD_Connection *pos)
mhd_assert (! pos->suspended);
mhd_assert (! pos->resuming);
- if (pos->connection_timeout == pos->daemon->connection_timeout)
+ if (pos->connection_timeout == daemon->connection_timeout)
XDLL_remove (daemon->normal_timeout_head,
daemon->normal_timeout_tail,
pos);
@@ -4433,6 +4442,8 @@ MHD_polling_thread (void *cls)
/* Resume any pending for resume connections, join
* all connection's threads (if any) and finally cleanup
* everything. */
+ if (0 != (MHD_TEST_ALLOW_SUSPEND_RESUME & daemon->options))
+ resume_suspended_connections (daemon);
close_all_connections (daemon);
return (MHD_THRD_RTRN_TYPE_)0;
@@ -6304,7 +6315,8 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
daemon->worker_pool[i].shutdown = true;
if (MHD_ITC_IS_VALID_(daemon->worker_pool[i].itc))
{
- if (! MHD_itc_activate_ (daemon->worker_pool[i].itc, "e"))
+ if (! MHD_itc_activate_ (daemon->worker_pool[i].itc,
+ "e"))
MHD_PANIC (_("Failed to signal shutdown via inter-thread communication channel."));
}
else
@@ -6335,11 +6347,8 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
if (0 != (daemon->options & MHD_USE_INTERNAL_POLLING_THREAD))
{ /* Worker daemon or single daemon with internal thread(s). */
mhd_assert (0 == daemon->worker_pool_size);
- if (0 != (MHD_TEST_ALLOW_SUSPEND_RESUME & daemon->options))
- resume_suspended_connections (daemon);
-
/* Separate thread(s) is used for polling sockets. */
- if (MHD_ITC_IS_VALID_(daemon->itc))
+ if (MHD_ITC_IS_VALID_ (daemon->itc))
{
if (! MHD_itc_activate_ (daemon->itc,
"e"))
diff --git a/src/microhttpd/test_upgrade.c b/src/microhttpd/test_upgrade.c
@@ -447,7 +447,8 @@ notify_completed_cb (void *cls,
{
pthread_t* ppth = *con_cls;
- (void)cls; (void)connection; /* Unused. Silent compiler warning. */
+ (void) cls;
+ (void) connection; /* Unused. Silent compiler warning. */
if ( (toe != MHD_REQUEST_TERMINATED_COMPLETED_OK) &&
(toe != MHD_REQUEST_TERMINATED_CLIENT_ABORT) &&
(toe != MHD_REQUEST_TERMINATED_DAEMON_SHUTDOWN) )
@@ -474,13 +475,14 @@ log_cb (void *cls,
const char *uri,
struct MHD_Connection *connection)
{
- pthread_t* ppth;
- (void)cls; (void)connection; /* Unused. Silent compiler warning. */
+ pthread_t *ppth;
+ (void) cls;
+ (void) connection; /* Unused. Silent compiler warning. */
if (0 != strcmp (uri,
"/"))
abort ();
- ppth = (pthread_t*) malloc (sizeof(pthread_t));
+ ppth = malloc (sizeof (pthread_t));
if (NULL == ppth)
abort();
*ppth = pthread_self ();
@@ -514,8 +516,9 @@ notify_connection_cb (void *cls,
enum MHD_ConnectionNotificationCode toe)
{
static int started;
- (void)cls; (void)connection; /* Unused. Silent compiler warning. */
+ (void) cls;
+ (void) connection; /* Unused. Silent compiler warning. */
switch (toe)
{
case MHD_CONNECTION_NOTIFY_STARTED:
@@ -773,7 +776,11 @@ upgrade_cb (void *cls,
MHD_socket sock,
struct MHD_UpgradeResponseHandle *urh)
{
- (void)cls; (void)connection; (void)con_cls; (void)extra_in; /* Unused. Silent compiler warning. */
+ (void) cls;
+ (void) connection;
+ (void) con_cls;
+ (void) extra_in; /* Unused. Silent compiler warning. */
+
usock = wr_create_from_plain_sckt (sock);
if (0 != extra_in_size)
abort ();
@@ -836,8 +843,12 @@ ahc_upgrade (void *cls,
{
struct MHD_Response *resp;
int ret;
- (void)cls;(void)url;(void)method; /* Unused. Silent compiler warning. */
- (void)version;(void)upload_data;(void)upload_data_size; /* Unused. Silent compiler warning. */
+ (void) cls;
+ (void) url;
+ (void) method; /* Unused. Silent compiler warning. */
+ (void) version;
+ (void) upload_data;
+ (void) upload_data_size; /* Unused. Silent compiler warning. */
if (NULL == *con_cls)
abort ();
@@ -1061,7 +1072,8 @@ test_upgrade (int flags,
{
#if defined(HTTPS_SUPPORT) && defined(HAVE_FORK) && defined(HAVE_WAITPID)
MHD_socket tls_fork_sock;
- if (-1 == (pid = gnutlscli_connect (&tls_fork_sock, dinfo->port)))
+ if (-1 == (pid = gnutlscli_connect (&tls_fork_sock,
+ dinfo->port)))
{
MHD_stop_daemon (d);
return 4;
@@ -1105,7 +1117,8 @@ main (int argc,
test_tls = has_in_name(argv[0], "_tls");
verbose = 1;
- if (has_param(argc, argv, "-q") || has_param(argc, argv, "--quiet"))
+ if (has_param(argc, argv, "-q") ||
+ has_param(argc, argv, "--quiet"))
verbose = 0;
if (test_tls)
@@ -1154,13 +1167,16 @@ main (int argc,
/* run tests */
if (verbose)
- printf ("Starting HTTP \"Upgrade\" tests with %s connections.\n", test_tls ? "TLS" : "plain");
+ printf ("Starting HTTP \"Upgrade\" tests with %s connections.\n",
+ test_tls ? "TLS" : "plain");
/* try external select */
res = test_upgrade (0,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with external select, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with external select, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with external select.\n");
@@ -1169,7 +1185,9 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with external 'auto', return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with external 'auto', return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with external 'auto'.\n");
@@ -1178,7 +1196,9 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with external select with EPOLL, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with external select with EPOLL, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with external select with EPOLL.\n");
#endif
@@ -1188,7 +1208,9 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with thread per connection, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with thread per connection, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with thread per connection.\n");
@@ -1196,7 +1218,9 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with thread per connection and 'auto', return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with thread per connection and 'auto', return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with thread per connection and 'auto'.\n");
#ifdef HAVE_POLL
@@ -1204,7 +1228,9 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with thread per connection and poll, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with thread per connection and poll, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with thread per connection and poll.\n");
#endif /* HAVE_POLL */
@@ -1214,28 +1240,36 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal select, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal select, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal select.\n");
res = test_upgrade (MHD_USE_INTERNAL_POLLING_THREAD,
2);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal select with thread pool, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal select with thread pool, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal select with thread pool.\n");
res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal 'auto' return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal 'auto' return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal 'auto'.\n");
res = test_upgrade (MHD_USE_AUTO | MHD_USE_INTERNAL_POLLING_THREAD,
2);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal 'auto' with thread pool, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal 'auto' with thread pool, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal 'auto' with thread pool.\n");
#ifdef HAVE_POLL
@@ -1243,13 +1277,17 @@ main (int argc,
0);
error_count += res;
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal poll, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal poll, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal poll.\n");
res = test_upgrade (MHD_USE_POLL_INTERNAL_THREAD,
2);
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal poll with thread pool, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal poll with thread pool, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal poll with thread pool.\n");
#endif
@@ -1257,13 +1295,17 @@ main (int argc,
res = test_upgrade (MHD_USE_EPOLL_INTERNAL_THREAD,
0);
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal epoll, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal epoll, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal epoll.\n");
res = test_upgrade (MHD_USE_EPOLL_INTERNAL_THREAD,
2);
if (res)
- fprintf (stderr, "FAILED: Upgrade with internal epoll, return code %d.\n", res);
+ fprintf (stderr,
+ "FAILED: Upgrade with internal epoll, return code %d.\n",
+ res);
else if (verbose)
printf ("PASSED: Upgrade with internal epoll.\n");
#endif
