Skip to content

Commit

Permalink
Do not advance the vs->vg pointer unless there was a match
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mbgrydeland committed Sep 9, 2019
1 parent e264e7e commit ec19cc1
Showing 1 changed file with 16 additions and 6 deletions.
22 changes: 16 additions & 6 deletions lib/libvarnishapi/vsm.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ec19cc1

Please sign in to comment.