[master] 217d927 serialize access to heritage.socks in the child

[Nils Goroll]

commit 217d9277fa4b6fc44a9d4c9b8736a937aa4029ba
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Mon Nov 6 17:51:23 2017 +0100

    serialize access to heritage.socks in the child
    
    I did consider avoiding locks, but as long as the close() in VCA_Shutdown()
    happens concurrent to vca_acct(), the file descriptor could become invalid
    at any time, so even if we'd not hit the sock > 0 assertion, other errors
    could occur / assertions be hit.
    
    We could also move the close to vca_acct() and just signal it to finish off,
    but this would delay the close by the sleep(1), unless we further complicated
    things with a condvar.
    
    Avoiding the lock would require us to make any operation on a socket fd
    ignore errors, which is totally against our policy to assert on everything.
    (see vca_tcp_opt_set() for example).
    
    So, for the time being, I think simple serialization using a pthread mutex
    is the safest and simplest solution. A varnish lock would be overkill, as
    this lock is really only relevant during VCA_Shutdown.
    
    IMHO, the primary risk involved here is delayed VCA_Shutdown() due to
    vca_acct() hanging in a syscall. Famous Last Words - sigh.
    
    Fixes #2482

diff --git a/bin/varnishd/cache/cache_acceptor.c b/bin/varnishd/cache/cache_acceptor.c
index f552b85..523bba9 100644
--- a/bin/varnishd/cache/cache_acceptor.c
+++ b/bin/varnishd/cache/cache_acceptor.c
@@ -53,6 +53,7 @@ static pthread_t	VCA_thread;
 static double vca_pace = 0.0;
 static struct lock pace_mtx;
 static unsigned pool_accepting;
+static pthread_mutex_t shut_mtx = PTHREAD_MUTEX_INITIALIZER;
 
 struct wrk_accept {
 	unsigned		magic;
@@ -529,9 +530,12 @@ vca_acct(void *arg)
 
 	(void)vca_tcp_opt_init();
 
+	AZ(pthread_mutex_lock(&shut_mtx));
 	VTAILQ_FOREACH(ls, &heritage.socks, list) {
 		CHECK_OBJ_NOTNULL(ls->transport, TRANSPORT_MAGIC);
-		assert (ls->sock > 0);		// We know where stdin is
+		if (ls->sock == -2)
+			continue;	// VCA_Shutdown
+		assert (ls->sock > 0);	// We know where stdin is
 		if (cache_param->tcp_fastopen) {
 			int i;
 			i = VTCP_fastopen(ls->sock, cache_param->listen_depth);
@@ -551,6 +555,7 @@ vca_acct(void *arg)
 				    ls->sock, i, strerror(errno));
 		}
 	}
+	AZ(pthread_mutex_unlock(&shut_mtx));
 
 	need_test = 1;
 	pool_accepting = 1;
@@ -559,12 +564,14 @@ vca_acct(void *arg)
 	while (1) {
 		(void)sleep(1);
 		if (vca_tcp_opt_init()) {
+			AZ(pthread_mutex_lock(&shut_mtx));
 			VTAILQ_FOREACH(ls, &heritage.socks, list) {
 				if (ls->sock == -2)
-					continue;	// raced VCA_Shutdown
+					continue;	// VCA_Shutdown
 				assert (ls->sock > 0);
 				vca_tcp_opt_set(ls->sock, 1);
 			}
+			AZ(pthread_mutex_unlock(&shut_mtx));
 		}
 		now = VTIM_real();
 		VSC_C_main->uptime = (uint64_t)(now - t0);
@@ -606,10 +613,12 @@ ccf_listen_address(struct cli *cli, const char * const *av, void *priv)
 	while (!pool_accepting)
 		VTIM_sleep(.1);
 
+	AZ(pthread_mutex_lock(&shut_mtx));
 	VTAILQ_FOREACH(ls, &heritage.socks, list) {
 		VTCP_myname(ls->sock, h, sizeof h, p, sizeof p);
 		VCLI_Out(cli, "%s %s\n", h, p);
 	}
+	AZ(pthread_mutex_unlock(&shut_mtx));
 }
 
 /*--------------------------------------------------------------------*/
@@ -634,11 +643,13 @@ VCA_Shutdown(void)
 	struct listen_sock *ls;
 	int i;
 
+	AZ(pthread_mutex_lock(&shut_mtx));
 	VTAILQ_FOREACH(ls, &heritage.socks, list) {
 		i = ls->sock;
 		ls->sock = -2;
 		(void)close(i);
 	}
+	AZ(pthread_mutex_unlock(&shut_mtx));
 }
 
 /*--------------------------------------------------------------------