[6.0] 8bd323591 Do not advance the vs->vg pointer unless there was a match

Martin Blix Grydeland martin at varnish-software.com
Fri Oct 18 13:23:07 UTC 2019


commit 8bd323591db8d555cf43be48528349f20b6d597c
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Sat Aug 31 13:22:23 2019 +0200

    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.

diff --git a/lib/libvarnishapi/vsm.c b/lib/libvarnishapi/vsm.c
index 92423ac5b..3fca11fb8 100644
--- a/lib/libvarnishapi/vsm.c
+++ b/lib/libvarnishapi/vsm.c
@@ -242,6 +242,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;
@@ -505,13 +510,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);


More information about the varnish-commit mailing list