From ec19cc19ec8eebd8b9816c7e8977995e6ff76778 Mon Sep 17 00:00:00 2001 From: Martin Blix Grydeland Date: Sat, 31 Aug 2019 13:22:23 +0200 Subject: [PATCH] Do not advance the vs->vg pointer unless there was a match The vsm_set vg pointer points to the last matched segment insertion, and is used as an optimization when looking for existing entries in the list. varnishd guarantees that insertions are always ordered, so there is no need to search the preceeding entries on a successful match. When parsing an index containg elements that included entries that are added and then subsequently removed in a later entry, this pointer would be advanced to the very end, failing to match the entries in between when parsing the rest of the file. Fix this mechanism by only updating the pointer on a successful match, and also advancing it one step it if the entry it is pointing to is removed. Note that these types of events are not supposed to be possible, because varnishd will when rewriting the index only include the active ones, removing any add-then-remove pairs. But if for whatever reason an attempt is made to reread a list, with this patch it should not cause problems. --- lib/libvarnishapi/vsm.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/libvarnishapi/vsm.c b/lib/libvarnishapi/vsm.c index d620899866..91540c8fd4 100644 --- a/lib/libvarnishapi/vsm.c +++ b/lib/libvarnishapi/vsm.c @@ -253,6 +253,11 @@ vsm_delseg(struct vsm_seg *vg, int refsok) CHECK_OBJ_NOTNULL(vg, VSM_SEG_MAGIC); + if (vg->set->vg == vg) { + AZ(vg->flags & VSM_FLAG_STALE); + vg->set->vg = VTAILQ_NEXT(vg, list); + } + if (refsok && vg->refs) { AZ(vg->flags & VSM_FLAG_STALE); vg->flags |= VSM_FLAG_STALE; @@ -516,13 +521,18 @@ vsm_vlu_plus(struct vsm *vd, struct vsm_set *vs, const char *line) return(-1); } - while (vs->vg != NULL && vsm_cmp_av(&vs->vg->av[1], &av[1])) - vs->vg = VTAILQ_NEXT(vs->vg, list); - if (vs->vg != NULL) { - VAV_Free(av); + vg = vs->vg; + CHECK_OBJ_ORNULL(vg, VSM_SEG_MAGIC); + if (vg != NULL) + AZ(vg->flags & VSM_FLAG_STALE); + while (vg != NULL && vsm_cmp_av(&vg->av[1], &av[1])) + vg = VTAILQ_NEXT(vg, list); + if (vg != NULL) { /* entry compared equal, so it survives */ - vs->vg->flags |= VSM_FLAG_MARKSCAN; - vs->vg = VTAILQ_NEXT(vs->vg, list); + CHECK_OBJ_NOTNULL(vg, VSM_SEG_MAGIC); + VAV_Free(av); + vg->flags |= VSM_FLAG_MARKSCAN; + vs->vg = VTAILQ_NEXT(vg, list); } else { ALLOC_OBJ(vg, VSM_SEG_MAGIC); AN(vg);