commit 2eb1573d56d2b44702c05650ee41fcbacf4b01ce
parent 2a815fe3330063bea19699729f9c6d6c2c836ebc
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 15 Feb 2017 13:25:50 +0100
fix race related to MHD_quiesce_daemon setting the listen socket to -1 which may disrupt concurrent non-locking activities by instead setting a flag (which suffices given the document semantics of MHD_quiesce_daemon()); renaming the socket_fd to listen_fd to distinguish it better by name
Diffstat:
2 files changed, 92 insertions(+), 66 deletions(-)
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
@@ -796,8 +796,9 @@ MHD_get_fdset2 (struct MHD_Daemon *daemon,
fd_setsize) ? MHD_YES : MHD_NO;
}
#endif
- ls = daemon->socket_fd;
+ ls = daemon->listen_fd;
if ( (MHD_INVALID_SOCKET != ls) &&
+ (! daemon->was_quiesced) &&
(! MHD_add_to_fd_set_ (ls,
read_fd_set,
max_fd,
@@ -2669,7 +2670,8 @@ MHD_accept_connection (struct MHD_Daemon *daemon)
memset (addr,
0,
sizeof (addrstorage));
- if (MHD_INVALID_SOCKET == (fd = daemon->socket_fd))
+ if ( (MHD_INVALID_SOCKET == (fd = daemon->listen_fd)) ||
+ (daemon->was_quiesced) )
return MHD_NO;
#ifdef USE_ACCEPT4
s = accept4 (fd,
@@ -3034,7 +3036,8 @@ MHD_run_from_select (struct MHD_Daemon *daemon,
#endif
/* select connection thread handling type */
- if ( (MHD_INVALID_SOCKET != (ds = daemon->socket_fd)) &&
+ if ( (MHD_INVALID_SOCKET != (ds = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) &&
(FD_ISSET (ds,
read_fd_set)) )
(void) MHD_accept_connection (daemon);
@@ -3144,7 +3147,8 @@ MHD_select (struct MHD_Daemon *daemon,
else
{
/* accept only, have one thread per connection */
- if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) &&
(! MHD_add_to_fd_set_ (ls,
&rs,
&maxsock,
@@ -3167,7 +3171,8 @@ MHD_select (struct MHD_Daemon *daemon,
/* fdset limit reached, new connections
cannot be handled. Remove listen socket FD
from fdset and retry to add ITC FD. */
- if (MHD_INVALID_SOCKET != (ls = daemon->socket_fd))
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) )
{
FD_CLR (ls,
&rs);
@@ -3193,7 +3198,7 @@ MHD_select (struct MHD_Daemon *daemon,
the shutdown OR the termination of an existing connection; so
only do this optimization if we have a signaling ITC in
place. */
- if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
(MHD_ITC_IS_VALID_(daemon->itc)) &&
(0 != (daemon->options & MHD_USE_ITC)) &&
( (daemon->connections == daemon->connection_limit) ||
@@ -3305,7 +3310,8 @@ MHD_poll_all (struct MHD_Daemon *daemon,
}
poll_server = 0;
poll_listen = -1;
- if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) &&
(daemon->connections < daemon->connection_limit) &&
(! daemon->at_limit) )
{
@@ -3516,7 +3522,9 @@ MHD_poll_listen_socket (struct MHD_Daemon *daemon,
poll_count = 0;
poll_listen = -1;
poll_itc_idx = -1;
- if (MHD_INVALID_SOCKET != (ls = daemon->socket_fd))
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) )
+
{
p[poll_count].fd = ls;
p[poll_count].events = POLLIN;
@@ -3716,7 +3724,8 @@ MHD_epoll (struct MHD_Daemon *daemon,
return MHD_NO; /* we're down! */
if (daemon->shutdown)
return MHD_NO;
- if ( (MHD_INVALID_SOCKET != (ls = daemon->socket_fd)) &&
+ if ( (MHD_INVALID_SOCKET != (ls = daemon->listen_fd)) &&
+ (! daemon->was_quiesced) &&
(daemon->connections < daemon->connection_limit) &&
(MHD_NO == daemon->listen_socket_in_epoll) &&
(! daemon->at_limit) )
@@ -3737,6 +3746,17 @@ MHD_epoll (struct MHD_Daemon *daemon,
}
daemon->listen_socket_in_epoll = MHD_YES;
}
+ if ( (daemon->was_quiesced) &&
+ (MHD_YES == daemon->listen_socket_in_epoll) )
+ {
+ if (0 != epoll_ctl (daemon->epoll_fd,
+ EPOLL_CTL_DEL,
+ ls,
+ NULL))
+ MHD_PANIC ("Failed to remove listen FD from epoll set\n");
+ daemon->listen_socket_in_epoll = MHD_NO;
+ }
+
#if defined(HTTPS_SUPPORT) && defined(UPGRADE_SUPPORT)
if ( (MHD_NO == daemon->upgrade_fd_in_epoll) &&
(-1 != daemon->epoll_upgrade_fd) )
@@ -3758,9 +3778,10 @@ MHD_epoll (struct MHD_Daemon *daemon,
daemon->upgrade_fd_in_epoll = MHD_YES;
}
#endif /* HTTPS_SUPPORT && UPGRADE_SUPPORT */
- if ( ( (MHD_YES == daemon->listen_socket_in_epoll) &&
- (daemon->connections == daemon->connection_limit) ) ||
- (daemon->at_limit) )
+ if ( (MHD_YES == daemon->listen_socket_in_epoll) &&
+ ( (daemon->connections == daemon->connection_limit) ||
+ (daemon->at_limit) ||
+ (daemon->was_quiesced) ) )
{
/* we're at the connection limit, disable listen socket
for event loop for now */
@@ -4166,7 +4187,7 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon)
unsigned int i;
MHD_socket ret;
- ret = daemon->socket_fd;
+ ret = daemon->listen_fd;
if (MHD_INVALID_SOCKET == ret)
return MHD_INVALID_SOCKET;
if ( (MHD_ITC_IS_INVALID_(daemon->itc)) &&
@@ -4182,7 +4203,7 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon)
if (NULL != daemon->worker_pool)
for (i = 0; i < daemon->worker_pool_size; i++)
{
- daemon->worker_pool[i].socket_fd = MHD_INVALID_SOCKET;
+ daemon->worker_pool[i].was_quiesced = true;
#ifdef EPOLL_SUPPORT
if ( (0 != (daemon->options & MHD_USE_EPOLL)) &&
(-1 != daemon->worker_pool[i].epoll_fd) &&
@@ -4203,10 +4224,7 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon)
MHD_PANIC (_("Failed to signal quiesce via inter-thread communication channel"));
}
}
- /* FIXME: This creates a race with the rest of the code.
- We may be adding the FD to the epoll-set concurrently
- in another thread! So we DO need to lock (yuck yuck). */
- daemon->socket_fd = MHD_INVALID_SOCKET;
+ daemon->was_quiesced = true;
#ifdef EPOLL_SUPPORT
if ( (0 != (daemon->options & MHD_USE_EPOLL)) &&
(-1 != daemon->epoll_fd) &&
@@ -4219,14 +4237,10 @@ MHD_quiesce_daemon (struct MHD_Daemon *daemon)
MHD_PANIC ("Failed to remove listen FD from epoll set\n");
daemon->listen_socket_in_epoll = MHD_NO;
}
- else
#endif
- if (MHD_ITC_IS_VALID_(daemon->itc))
- {
- if (! MHD_itc_activate_ (daemon->itc, "q"))
- MHD_PANIC (_("failed to signal quiesce via inter-thread communication channel"));
- }
-
+ if ( (MHD_ITC_IS_VALID_(daemon->itc)) &&
+ (! MHD_itc_activate_ (daemon->itc, "q")) )
+ MHD_PANIC (_("failed to signal quiesce via inter-thread communication channel"));
return ret;
}
@@ -4499,7 +4513,7 @@ parse_options_va (struct MHD_Daemon *daemon,
break;
#endif
case MHD_OPTION_LISTEN_SOCKET:
- daemon->socket_fd = va_arg (ap,
+ daemon->listen_fd = va_arg (ap,
MHD_socket);
break;
case MHD_OPTION_EXTERNAL_LOGGER:
@@ -4721,7 +4735,8 @@ setup_epoll_to_listen (struct MHD_Daemon *daemon)
return MHD_NO;
}
#endif /* HTTPS_SUPPORT && UPGRADE_SUPPORT */
- if (MHD_INVALID_SOCKET == (ls = daemon->socket_fd))
+ if ( (MHD_INVALID_SOCKET == (ls = daemon->listen_fd)) ||
+ (daemon->was_quiesced) )
return MHD_YES; /* non-listening daemon */
event.events = EPOLLIN;
event.data.ptr = daemon;
@@ -4789,7 +4804,7 @@ MHD_start_daemon_va (unsigned int flags,
{
const MHD_SCKT_OPT_BOOL_ on = 1;
struct MHD_Daemon *daemon;
- MHD_socket socket_fd;
+ MHD_socket listen_fd;
struct sockaddr_in servaddr4;
#if HAVE_INET6
struct sockaddr_in6 servaddr6;
@@ -4887,7 +4902,7 @@ MHD_start_daemon_va (unsigned int flags,
NULL);
}
#endif /* HTTPS_SUPPORT */
- daemon->socket_fd = MHD_INVALID_SOCKET;
+ daemon->listen_fd = MHD_INVALID_SOCKET;
daemon->listening_address_reuse = 0;
daemon->options = flags;
daemon->port = port;
@@ -5052,12 +5067,12 @@ MHD_start_daemon_va (unsigned int flags,
goto free_and_fail;
}
#endif
- if ( (MHD_INVALID_SOCKET == daemon->socket_fd) &&
+ if ( (MHD_INVALID_SOCKET == daemon->listen_fd) &&
(0 == (daemon->options & MHD_USE_NO_LISTEN_SOCKET)) )
{
/* try to open listen socket */
- socket_fd = MHD_socket_create_listen_(flags & MHD_USE_IPv6);
- if (MHD_INVALID_SOCKET == socket_fd)
+ listen_fd = MHD_socket_create_listen_(flags & MHD_USE_IPv6);
+ if (MHD_INVALID_SOCKET == listen_fd)
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
@@ -5075,7 +5090,7 @@ MHD_start_daemon_va (unsigned int flags,
* on non-W32 platforms, and do not fail if it doesn't work.
* Don't use it on W32, because on W32 it will allow multiple
* bind to the same address:port, like SO_REUSEPORT on others. */
- if (0 > setsockopt (socket_fd,
+ if (0 > setsockopt (listen_fd,
SOL_SOCKET,
SO_REUSEADDR,
(void*)&on, sizeof (on)))
@@ -5094,7 +5109,7 @@ MHD_start_daemon_va (unsigned int flags,
#ifndef _WIN32
/* Use SO_REUSEADDR on non-W32 platforms, and do not fail if
* it doesn't work. */
- if (0 > setsockopt (socket_fd,
+ if (0 > setsockopt (listen_fd,
SOL_SOCKET,
SO_REUSEADDR,
(void*)&on, sizeof (on)))
@@ -5112,7 +5127,7 @@ MHD_start_daemon_va (unsigned int flags,
/* SO_REUSEADDR on W32 has the same semantics
as SO_REUSEPORT on BSD/Linux */
#if defined(_WIN32) || defined(SO_REUSEPORT)
- if (0 > setsockopt (socket_fd,
+ if (0 > setsockopt (listen_fd,
SOL_SOCKET,
#ifndef _WIN32
SO_REUSEPORT,
@@ -5150,7 +5165,7 @@ MHD_start_daemon_va (unsigned int flags,
#if (defined(_WIN32) && defined(SO_EXCLUSIVEADDRUSE)) || \
(defined(__sun) && defined(SO_EXCLBIND))
#ifdef SO_EXCLUSIVEADDRUSE
- if (0 > setsockopt (socket_fd,
+ if (0 > setsockopt (listen_fd,
SOL_SOCKET,
#ifdef SO_EXCLUSIVEADDRUSE
SO_EXCLUSIVEADDRUSE,
@@ -5213,7 +5228,7 @@ MHD_start_daemon_va (unsigned int flags,
servaddr = (struct sockaddr *) &servaddr4;
}
}
- daemon->socket_fd = socket_fd;
+ daemon->listen_fd = listen_fd;
if (0 != (flags & MHD_USE_IPv6))
{
@@ -5225,7 +5240,7 @@ MHD_start_daemon_va (unsigned int flags,
your IPv6 socket may then also bind against IPv4 anyway... */
const MHD_SCKT_OPT_BOOL_ v6_only =
(MHD_USE_DUAL_STACK != (flags & MHD_USE_DUAL_STACK));
- if (0 > setsockopt (socket_fd,
+ if (0 > setsockopt (listen_fd,
IPPROTO_IPV6, IPV6_V6ONLY,
(const void *) &v6_only,
sizeof (v6_only)))
@@ -5239,7 +5254,7 @@ MHD_start_daemon_va (unsigned int flags,
#endif
#endif
}
- if (-1 == bind (socket_fd, servaddr, addrlen))
+ if (-1 == bind (listen_fd, servaddr, addrlen))
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
@@ -5247,7 +5262,7 @@ MHD_start_daemon_va (unsigned int flags,
(unsigned int) port,
MHD_socket_last_strerr_ ());
#endif
- MHD_socket_close_chk_ (socket_fd);
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
#ifdef TCP_FASTOPEN
@@ -5255,7 +5270,7 @@ MHD_start_daemon_va (unsigned int flags,
{
if (0 == daemon->fastopen_queue_size)
daemon->fastopen_queue_size = MHD_TCP_FASTOPEN_QUEUE_SIZE_DEFAULT;
- if (0 != setsockopt (socket_fd,
+ if (0 != setsockopt (listen_fd,
IPPROTO_TCP,
TCP_FASTOPEN,
&daemon->fastopen_queue_size,
@@ -5269,7 +5284,7 @@ MHD_start_daemon_va (unsigned int flags,
}
}
#endif
- if (listen (socket_fd,
+ if (listen (listen_fd,
daemon->listen_backlog_size) < 0)
{
#ifdef HAVE_MESSAGES
@@ -5277,16 +5292,16 @@ MHD_start_daemon_va (unsigned int flags,
_("Failed to listen for connections: %s\n"),
MHD_socket_last_strerr_ ());
#endif
- MHD_socket_close_chk_ (socket_fd);
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
}
else
{
- socket_fd = daemon->socket_fd;
+ listen_fd = daemon->listen_fd;
}
- if (!MHD_socket_nonblocking_ (socket_fd))
+ if (!MHD_socket_nonblocking_ (listen_fd))
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
@@ -5299,21 +5314,21 @@ MHD_start_daemon_va (unsigned int flags,
/* Accept must be non-blocking. Multiple children may wake up
* to handle a new connection, but only one will win the race.
* The others must immediately return. */
- MHD_socket_close_chk_ (socket_fd);
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
}
- if ( (!MHD_SCKT_FD_FITS_FDSET_(socket_fd,
+ if ( (!MHD_SCKT_FD_FITS_FDSET_(listen_fd,
NULL)) &&
(0 == (flags & (MHD_USE_POLL | MHD_USE_EPOLL)) ) )
{
#ifdef HAVE_MESSAGES
MHD_DLOG (daemon,
_("Socket descriptor larger than FD_SETSIZE: %d > %d\n"),
- socket_fd,
+ listen_fd,
FD_SETSIZE);
#endif
- MHD_socket_close_chk_ (socket_fd);
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
@@ -5341,8 +5356,8 @@ MHD_start_daemon_va (unsigned int flags,
MHD_DLOG (daemon,
_("MHD failed to initialize IP connection limit mutex\n"));
#endif
- if (MHD_INVALID_SOCKET != socket_fd)
- MHD_socket_close_chk_ (socket_fd);
+ if (MHD_INVALID_SOCKET != listen_fd)
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
if (! MHD_mutex_init_ (&daemon->cleanup_connection_mutex))
@@ -5352,8 +5367,8 @@ MHD_start_daemon_va (unsigned int flags,
_("MHD failed to initialize IP connection limit mutex\n"));
#endif
MHD_mutex_destroy_chk_ (&daemon->cleanup_connection_mutex);
- if (MHD_INVALID_SOCKET != socket_fd)
- MHD_socket_close_chk_ (socket_fd);
+ if (MHD_INVALID_SOCKET != listen_fd)
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
@@ -5366,8 +5381,8 @@ MHD_start_daemon_va (unsigned int flags,
MHD_DLOG (daemon,
_("Failed to initialize TLS support\n"));
#endif
- if (MHD_INVALID_SOCKET != socket_fd)
- MHD_socket_close_chk_ (socket_fd);
+ if (MHD_INVALID_SOCKET != listen_fd)
+ MHD_socket_close_chk_ (listen_fd);
MHD_mutex_destroy_chk_ (&daemon->cleanup_connection_mutex);
MHD_mutex_destroy_chk_ (&daemon->per_ip_connection_mutex);
goto free_and_fail;
@@ -5390,8 +5405,8 @@ MHD_start_daemon_va (unsigned int flags,
#endif
MHD_mutex_destroy_chk_ (&daemon->cleanup_connection_mutex);
MHD_mutex_destroy_chk_ (&daemon->per_ip_connection_mutex);
- if (MHD_INVALID_SOCKET != socket_fd)
- MHD_socket_close_chk_ (socket_fd);
+ if (MHD_INVALID_SOCKET != listen_fd)
+ MHD_socket_close_chk_ (listen_fd);
goto free_and_fail;
}
if ( (daemon->worker_pool_size > 0) &&
@@ -5507,8 +5522,8 @@ thread_failed:
MHD_USE_INTERNAL_POLLING_THREAD mode. */
if (0 == i)
{
- if (MHD_INVALID_SOCKET != socket_fd)
- MHD_socket_close_chk_ (socket_fd);
+ if (MHD_INVALID_SOCKET != listen_fd)
+ MHD_socket_close_chk_ (listen_fd);
MHD_mutex_destroy_chk_ (&daemon->cleanup_connection_mutex);
MHD_mutex_destroy_chk_ (&daemon->per_ip_connection_mutex);
if (NULL != daemon->worker_pool)
@@ -5728,7 +5743,7 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
resume_suspended_connections (daemon);
daemon->shutdown = true;
- fd = daemon->socket_fd;
+ fd = daemon->listen_fd;
if (0 != (daemon->options & MHD_USE_INTERNAL_POLLING_THREAD))
{
@@ -5750,7 +5765,8 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
{
/* fd might be MHD_INVALID_SOCKET here due to 'MHD_quiesce_daemon' */
/* No problem if shutdown will be called several times for the same socket. */
- (void) shutdown (fd, SHUT_RDWR);
+ (void) shutdown (fd,
+ SHUT_RDWR);
}
#endif
}
@@ -5785,8 +5801,10 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
else
{
/* fd might be MHD_INVALID_SOCKET here due to 'MHD_quiesce_daemon' */
- if (MHD_INVALID_SOCKET != fd)
- (void) shutdown (fd, SHUT_RDWR);
+ if ( (MHD_INVALID_SOCKET != fd) &&
+ (! daemon->was_quiesced) )
+ (void) shutdown (fd,
+ SHUT_RDWR);
}
#endif
if (! MHD_join_thread_ (daemon->pid))
@@ -5801,7 +5819,8 @@ MHD_stop_daemon (struct MHD_Daemon *daemon)
close_all_connections (daemon);
}
- if (MHD_INVALID_SOCKET != fd)
+ if ( (MHD_INVALID_SOCKET != fd) &&
+ (! daemon->was_quiesced) )
MHD_socket_close_chk_ (fd);
if (MHD_ITC_IS_VALID_ (daemon->itc))
@@ -5867,7 +5886,7 @@ MHD_get_daemon_info (struct MHD_Daemon *daemon,
case MHD_DAEMON_INFO_MAC_KEY_SIZE:
return NULL; /* no longer supported */
case MHD_DAEMON_INFO_LISTEN_FD:
- return (const union MHD_DaemonInfo *) &daemon->socket_fd;
+ return (const union MHD_DaemonInfo *) &daemon->listen_fd;
#ifdef EPOLL_SUPPORT
case MHD_DAEMON_INFO_EPOLL_FD:
return (const union MHD_DaemonInfo *) &daemon->epoll_fd;
diff --git a/src/microhttpd/internal.h b/src/microhttpd/internal.h
@@ -1374,7 +1374,7 @@ struct MHD_Daemon
/**
* Listen socket.
*/
- MHD_socket socket_fd;
+ MHD_socket listen_fd;
/**
* Whether to allow/disallow/ignore reuse of listening address.
@@ -1426,6 +1426,13 @@ struct MHD_Daemon
volatile bool shutdown;
/**
+ * Has this deamon been quiesced via #MHD_quiesce_daemon()?
+ * If so, we should no longer use the @e listen_fd (including
+ * removing it from the @e epoll_fd when possible).
+ */
+ volatile bool was_quiesced;
+
+ /**
* Did we hit some system or process-wide resource limit while
* trying to accept() the last time? If so, we don't accept new
* connections until we close an existing one. This effectively
