[master] f8936e78a Change the panic reentrancy check so that Coverity can (hopefully) grok it.

Poul-Henning Kamp phk at FreeBSD.org
Mon Dec 4 08:56:10 UTC 2023


commit f8936e78a50153f3203bb077be8a067eac0c9663
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Dec 4 08:53:22 2023 +0000

    Change the panic reentrancy check so that Coverity can (hopefully) grok it.

diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index 3e093c50b..7b433c596 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -81,6 +81,7 @@ static pthread_key_t req_key;
 static pthread_key_t bo_key;
 static pthread_key_t wrk_key;
 pthread_key_t witness_key;
+pthread_key_t panic_key;
 
 void
 THR_SetBusyobj(const struct busyobj *bo)
@@ -404,6 +405,7 @@ child_main(int sigmagic, size_t altstksz)
 	PTOK(pthread_key_create(&wrk_key, NULL));
 	PTOK(pthread_key_create(&witness_key, free));
 	PTOK(pthread_key_create(&name_key, NULL));
+	PTOK(pthread_key_create(&panic_key, NULL));
 
 	THR_SetName("cache-main");
 
diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index 221d9835c..eca20c32b 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -67,7 +67,6 @@
 
 static struct vsb pan_vsb_storage, *pan_vsb;
 static pthread_mutex_t panicstr_mtx;
-static pthread_t panicy;
 
 static void pan_sess(struct vsb *, const struct sess *);
 static void pan_req(struct vsb *, const struct req *);
@@ -744,18 +743,23 @@ pan_ic(const char *func, const char *file, int line, const char *cond,
 	struct busyobj *bo;
 	struct worker *wrk;
 	struct sigaction sa;
-	int err = errno;
+	int i, err = errno;
 
-	/* If we already panicing in another thread, do nothing */
-	while (heritage.panic_str[0] && panicy != pthread_self())
-		sleep(1);
-
-	if (pthread_mutex_lock(&panicstr_mtx)) {
-		/* Reentrant panic */
+	if (pthread_getspecific(panic_key) != NULL) {
 		VSB_cat(pan_vsb, "\n\nPANIC REENTRANCY\n\n");
 		abort();
 	}
-	panicy = pthread_self();
+
+	/* If we already panicing in another thread, do nothing */
+	do {
+		i = pthread_mutex_trylock(&panicstr_mtx);
+		if (i != 0)
+			sleep (1);
+	} while (i != 0);
+
+	assert (VSB_len(pan_vsb) == 0);
+
+	AZ(pthread_setspecific(panic_key, pan_vsb));
 
 	/*
 	 * should we trigger a SIGSEGV while handling a panic, our sigsegv
@@ -844,6 +848,14 @@ pan_ic(const char *func, const char *file, int line, const char *cond,
 	VSB_putc(pan_vsb, '\0');	/* NUL termination */
 
 	v_gcov_flush();
+
+	/*
+	 * Do a little song and dance for static checkers which
+	 * are not smart enough to figure out that calling abort()
+	 * with a mutex held is OK and probably very intentional.
+	 */
+	if (pthread_getspecific(panic_key))	/* ie: always */
+		abort();
 	PTOK(pthread_mutex_unlock(&panicstr_mtx));
 	abort();
 }
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index b534eb174..e9ffad17c 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -303,6 +303,7 @@ unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*);
 
 /* cache_main.c */
 vxid_t VXID_Get(const struct worker *, uint64_t marker);
+extern pthread_key_t panic_key;
 extern pthread_key_t witness_key;
 
 void THR_SetName(const char *name);


More information about the varnish-commit mailing list