This is to give persistent stevedores a chance to clean up their ban
lists without having to worry about the lists changing under them.
---
bin/varnishd/cache/cache.h | 3 +-
bin/varnishd/cache/cache_ban.c | 66 ++++++++++++++++++++++++++++++++++++---
bin/varnishd/cache/cache_main.c | 1 +
bin/varnishd/cache/cache_vrt.c | 10 +++---
4 files changed, 68 insertions(+), 12 deletions(-)
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 727073b..2571f89 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -746,8 +746,9 @@ struct ban *BAN_New(void);
int BAN_AddTest(struct cli *, struct ban *, const char *, const char *,
const char *);
void BAN_Free(struct ban *b);
-void BAN_Insert(struct ban *b);
+int BAN_Insert(struct ban *b);
void BAN_Init(void);
+void BAN_Shutdown(void);
void BAN_NewObjCore(struct objcore *oc);
void BAN_DestroyObj(struct objcore *oc);
int BAN_CheckObject(struct object *o, struct req *sp);
diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index 47a8518..66369d8 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -105,6 +105,7 @@ static struct ban *ban_magic;
static pthread_t ban_thread;
static struct ban * volatile ban_start;
static bgthread_t ban_lurker;
+static int ban_shutdown = 0;
/*--------------------------------------------------------------------
* BAN string defines & magic markers
@@ -406,9 +407,14 @@ BAN_AddTest(struct cli *cli, struct ban *b, const char
*a1, const char *a2,
* as a separate variable from the VTAILQ, to avoid depending on the
* internals of the VTAILQ macros. We tacitly assume that a pointer
* write is always atomic in doing so.
+ *
+ * Returns:
+ * 0: Ban successfully inserted
+ * -1: Ban not inserted due to shutdown in progress. The ban has been
+ * deleted.
*/
-void
+int
BAN_Insert(struct ban *b)
{
struct ban *bi, *be;
@@ -417,6 +423,11 @@ BAN_Insert(struct ban *b)
CHECK_OBJ_NOTNULL(b, BAN_MAGIC);
+ if (ban_shutdown) {
+ BAN_Free(b);
+ return (-1);
+ }
+
AZ(VSB_finish(b->vsb));
ln = VSB_len(b->vsb);
assert(ln >= 0);
@@ -435,6 +446,12 @@ BAN_Insert(struct ban *b)
b->vsb = NULL;
Lck_Lock(&ban_mtx);
+ if (ban_shutdown) {
+ /* Check again, we might have raced */
+ Lck_Unlock(&ban_mtx);
+ BAN_Free(b);
+ return (-1);
+ }
VTAILQ_INSERT_HEAD(&ban_head, b, list);
ban_start = b;
VSC_C_main->bans++;
@@ -452,12 +469,12 @@ BAN_Insert(struct ban *b)
Lck_Unlock(&ban_mtx);
if (be == NULL)
- return;
+ return (0);
/* Hunt down duplicates, and mark them as gone */
bi = b;
Lck_Lock(&ban_mtx);
- while(bi != be) {
+ while(!ban_shutdown && bi != be) {
bi = VTAILQ_NEXT(bi, list);
if (bi->flags & BAN_F_GONE)
continue;
@@ -468,6 +485,8 @@ BAN_Insert(struct ban *b)
}
be->refcount--;
Lck_Unlock(&ban_mtx);
+
+ return (0);
}
/*--------------------------------------------------------------------
@@ -551,6 +570,7 @@ BAN_Reload(const uint8_t *ban, unsigned len)
double t0, t1, t2 = 9e99;
ASSERT_CLI();
+ AZ(ban_shutdown);
t0 = ban_time(ban);
assert(len == ban_len(ban));
@@ -628,6 +648,7 @@ BAN_Compile(void)
{
ASSERT_CLI();
+ AZ(ban_shutdown);
/* Notify stevedores */
STV_BanInfo(BI_NEW, ban_magic->spec, ban_len(ban_magic->spec));
@@ -874,6 +895,8 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl,
unsigned pass)
ban_time(b->spec), b->refcount);
while (1) {
Lck_Lock(&ban_mtx);
+ if (ban_shutdown)
+ break;
oc = VTAILQ_FIRST(&b->objcore);
if (oc == NULL)
break;
@@ -957,6 +980,8 @@ ban_lurker_work(struct worker *wrk, struct vsl_log *vsl,
unsigned pass)
ban_time(b->spec));
}
Lck_Unlock(&ban_mtx);
+ if (ban_shutdown)
+ break;
VTIM_sleep(cache_param->ban_lurker_sleep);
if (b == b0)
break;
@@ -975,7 +1000,7 @@ ban_lurker(struct worker *wrk, void *priv)
VSL_Setup(&vsl, NULL, 0);
(void)priv;
- while (1) {
+ while (!ban_shutdown) {
ban_sleep = 1.0;
/* Clear any gone last bans */
@@ -1004,8 +1029,14 @@ ban_lurker(struct worker *wrk, void *priv)
WRK_SumStat(wrk);
}
+ if (ban_shutdown)
+ break;
+
VTIM_sleep(ban_sleep);
}
+
+ pthread_exit(0);
+
NEEDLESS_RETURN(NULL);
}
@@ -1048,7 +1079,11 @@ ccf_ban(struct cli *cli, const char * const *av, void
*priv)
BAN_Free(b);
return;
}
- BAN_Insert(b);
+ if (BAN_Insert(b) < 0) {
+ VCLI_Out(cli, "Shutdown in progress");
+ VCLI_SetResult(cli, CLIS_CANT);
+ return;
+ }
}
static void
@@ -1160,3 +1195,24 @@ BAN_Init(void)
VSC_C_main->bans_gone++;
BAN_Insert(ban_magic);
}
+
+/*--------------------------------------------------------------------
+ * Shutdown of the ban system.
+ *
+ * When this function returns, no new bans will be accepted, and no
+ * bans will be dropped (ban lurker thread stopped), so that no
+ * STV_BanInfo calls will be executed.
+ */
+
+void
+BAN_Shutdown(void)
+{
+ void *status;
+
+ Lck_Lock(&ban_mtx);
+ ban_shutdown = 1;
+ Lck_Unlock(&ban_mtx);
+
+ AZ(pthread_join(ban_thread, &status));
+ AZ(status);
+}
diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index 468401b..13c6612 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -232,6 +232,7 @@ child_main(void)
CLI_Run();
+ BAN_Shutdown();
STV_close();
printf("Child dies\n");
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index ed7ae59..914531e 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -459,10 +459,9 @@ VRT_ban(const struct req *req, char *cmds, ...)
good = 1;
}
if (!good)
- /* XXX: report error how ? */
- BAN_Free(b);
+ BAN_Free(b); /* XXX: report error how ? */
else
- BAN_Insert(b);
+ (void)BAN_Insert(b); /* XXX: report error how ? */
}
/*--------------------------------------------------------------------*/
@@ -506,10 +505,9 @@ VRT_ban_string(const struct req *req, const char *str)
break;
}
if (!good)
- /* XXX: report error how ? */
- BAN_Free(b);
+ BAN_Free(b); /* XXX: report error how ? */
else
- BAN_Insert(b);
+ (void)BAN_Insert(b); /* XXX: report error how ? */
VAV_Free(av);
}
--
1.7.9.5
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
