From aadb0ac761f7cac0c51088db8f753320324873c0 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Thu, 7 Mar 2024 20:59:12 +0100 Subject: [PATCH 1/3] Stabilize tests --- bin/varnishtest/tests/r01506.vtc | 1 + bin/varnishtest/tests/r02035.vtc | 1 + 2 files changed, 2 insertions(+) diff --git a/bin/varnishtest/tests/r01506.vtc b/bin/varnishtest/tests/r01506.vtc index 96b7b54c9a..02d70d75ac 100644 --- a/bin/varnishtest/tests/r01506.vtc +++ b/bin/varnishtest/tests/r01506.vtc @@ -68,3 +68,4 @@ client c1 { } -run varnish v1 -expect sc_range_short == 0 +server s1 -wait diff --git a/bin/varnishtest/tests/r02035.vtc b/bin/varnishtest/tests/r02035.vtc index eff24620c7..6f7f6dfd3e 100644 --- a/bin/varnishtest/tests/r02035.vtc +++ b/bin/varnishtest/tests/r02035.vtc @@ -34,3 +34,4 @@ client c1 { barrier b1 sync } -run +server s1 -wait From 14362fae7e1cb9cc2b6ea8e5ff42dbc68f9ef1c5 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Thu, 7 Mar 2024 20:55:28 +0100 Subject: [PATCH 2/3] Add VCL_Shutdown to wait for VCL references to vanish Before shutting down stevedores, we should wait for VCL references to go away to, indirectly, ensure that all worker threads potentially using storage functions are done. See https://gitlab.com/uplex/varnish/slash/-/issues/61 --- bin/varnishd/cache/cache_main.c | 17 +++++++++++++++++ bin/varnishd/cache/cache_varnishd.h | 1 + bin/varnishd/cache/cache_vcl.c | 22 ++++++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c index d29d3d3aec..1f62e8a5e6 100644 --- a/bin/varnishd/cache/cache_main.c +++ b/bin/varnishd/cache/cache_main.c @@ -362,6 +362,20 @@ cli_quit(int sig) WRONG("It's time for the big quit"); } +/*===================================================================== + * XXX Generalize? + */ + +static void +bit_set(volatile uint8_t *p, unsigned no) +{ + uint8_t b; + + p += (no >> 3); + b = (0x80 >> (no & 7)); + *p |= b; +} + /*===================================================================== * Run the child process */ @@ -452,12 +466,15 @@ child_main(int sigmagic, size_t altstksz) CLI_Run(); + bit_set(cache_param->debug_bits, DBG_VCLREL); + if (shutdown_delay > 0) VTIM_sleep(shutdown_delay); VCA_Shutdown(); BAN_Shutdown(); EXP_Shutdown(); + VCL_Shutdown(); STV_close(); printf("Child dies\n"); diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index add300e76c..4eab193764 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -491,6 +491,7 @@ struct vsb *VCL_Rel_CliCtx(struct vrt_ctx **); void VCL_Panic(struct vsb *, const char *nm, const struct vcl *); void VCL_Poll(void); void VCL_Init(void); +void VCL_Shutdown(void); #define VCL_MET_MAC(l,u,t,b) \ void VCL_##l##_method(struct vcl *, struct worker *, struct req *, \ diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c index 90e41ece8f..fd7a9fa58e 100644 --- a/bin/varnishd/cache/cache_vcl.c +++ b/bin/varnishd/cache/cache_vcl.c @@ -1027,3 +1027,25 @@ VCL_Init(void) Lck_New(&vcl_mtx, lck_vcl); VSL_Setup(&vsl_cli, NULL, 0); } + +void +VCL_Shutdown(void) +{ + struct vcl *vcl; + unsigned c = 0; + + while (1) { + Lck_Lock(&vcl_mtx); + VTAILQ_FOREACH(vcl, &vcl_head, list) + if (vcl->busy) + break; + Lck_Unlock(&vcl_mtx); + if (vcl == NULL) + return; + if (++c % 10 == 0) { + fprintf(stderr, "shutdown waiting for vcl %u\t%s\n", + vcl->busy, vcl->loaded_name); + } + usleep(100 * 1000); + } +} From bcd73bfc3a51edc58157c2a29f9e63a3b3faec48 Mon Sep 17 00:00:00 2001 From: Nils Goroll Date: Thu, 7 Mar 2024 21:09:10 +0100 Subject: [PATCH 3/3] Excecise the two-step stevedore closing before/after waiting for VCL ... references to vanish --- bin/varnishd/cache/cache_main.c | 1 + bin/varnishd/cache/cache_varnishd.h | 1 + bin/varnishd/storage/stevedore.c | 23 ++++++++++++++++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c index 1f62e8a5e6..d3e44fbcaf 100644 --- a/bin/varnishd/cache/cache_main.c +++ b/bin/varnishd/cache/cache_main.c @@ -474,6 +474,7 @@ child_main(int sigmagic, size_t altstksz) VCA_Shutdown(); BAN_Shutdown(); EXP_Shutdown(); + STV_warn(); VCL_Shutdown(); STV_close(); diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index 4eab193764..9063ba35a0 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -570,6 +570,7 @@ void V2D_Init(void); /* stevedore.c */ void STV_open(void); +void STV_warn(void); void STV_close(void); const struct stevedore *STV_next(void); int STV_BanInfoDrop(const uint8_t *ban, unsigned len); diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c index 34f3919fef..7ac6083abf 100644 --- a/bin/varnishd/storage/stevedore.c +++ b/bin/varnishd/storage/stevedore.c @@ -198,19 +198,28 @@ STV_open(void) AN(stv_h2_rxbuf); } +void +STV_warn(void) +{ + struct stevedore *stv; + + ASSERT_CLI(); + /* First send close warning */ + STV_Foreach(stv) + if (stv->close != NULL) + stv->close(stv, 0); +} + void STV_close(void) { struct stevedore *stv; - int i; ASSERT_CLI(); - for (i = 1; i >= 0; i--) { - /* First send close warning */ - STV_Foreach(stv) - if (stv->close != NULL) - stv->close(stv, i); - } + /* First send close warning */ + STV_Foreach(stv) + if (stv->close != NULL) + stv->close(stv, 1); } /*-------------------------------------------------------------------