[master] 84d1fc92f vai: fix lease leak
Nils Goroll
nils.goroll at uplex.de
Sun Jul 6 14:48:05 UTC 2025
commit 84d1fc92f436e9c94d127ec45e779b36f0f7c2a7
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Sun Jul 6 15:33:56 2025 +0200
vai: fix lease leak
since the merge, the great diversity of our vtest machines exposed a vai lease
leak via c00111.vtc:
In storage_simple.c, we can not free the last struct storage in the storage list
before we potentially advanced to the next. The previous code was failing to
return it.
We now use a null_iov pointer with a zero length viov to pass the lease to the
iterator with the only purpose to return it.
diff --git a/bin/varnishd/storage/storage_simple.c b/bin/varnishd/storage/storage_simple.c
index 23b6d2564..6ea4821f8 100644
--- a/bin/varnishd/storage/storage_simple.c
+++ b/bin/varnishd/storage/storage_simple.c
@@ -48,6 +48,8 @@
// marker pointer for sml_trimstore
static void *trim_once = &trim_once;
+// for delayed return of hdl->last resume pointer
+static void *null_iov = &null_iov;
/*-------------------------------------------------------------------*/
@@ -496,23 +498,16 @@ sml_ai_lease_boc(struct worker *wrk, vai_hdl vhdl, struct vscarab *scarab)
assert(state < BOS_FINISHED);
}
Lck_Lock(&hdl->boc->mtx);
- if (hdl->st == NULL && hdl->last != NULL) {
- /* when the "last" st completed, we did not yet have a next, so
- * resume from there. Because "last" might have been returned and
- * deleted, we can not just use the pointer, but rather need to
- * iterate the st list.
- * if we can not find "last", it also has been returned and
- * deleted, and the current write head (VTAILQ_LAST) is our next
- * st, which can also be null if we are done.
- */
- VTAILQ_FOREACH_REVERSE(next, &hdl->obj->list, storagehead, list) {
- if (next == hdl->last) {
- hdl->st = VTAILQ_PREV(next, storagehead, list);
- break;
- }
- }
+ if (hdl->st == NULL && hdl->last != NULL)
+ hdl->st = VTAILQ_PREV(hdl->last, storagehead, list);
+ if (hdl->last != NULL && state < BOS_FINISHED) {
+ viov = VSCARAB_GET(scarab);
+ AN(viov);
+ viov->iov.iov_base = null_iov;
+ viov->iov.iov_len = 0;
+ viov->lease = st2lease(hdl->last);
+ hdl->last = NULL;
}
- hdl->last = NULL;
if (hdl->st == NULL) {
assert(hdl->returned == 0 || hdl->avail == hdl->returned);
hdl->st = VTAILQ_LAST(&hdl->obj->list, storagehead);
@@ -533,15 +528,18 @@ sml_ai_lease_boc(struct worker *wrk, vai_hdl vhdl, struct vscarab *scarab)
if (hdl->st_off + l == hdl->st->space) {
next = VTAILQ_PREV(hdl->st, storagehead, list);
AZ(hdl->last);
- if (next == NULL)
+ if (next == NULL) {
hdl->last = hdl->st;
- else
+ viov->lease = VAI_LEASE_NORET;
+ }
+ else {
CHECK_OBJ(next, STORAGE_MAGIC);
+ viov->lease = st2lease(hdl->st);
+ }
#ifdef VAI_DBG
VSLb(wrk->vsl, SLT_Debug, "off %zu + l %zu == space st %p next st %p stvprv %p",
hdl->st_off, l, hdl->st, next, hdl->boc->stevedore_priv);
#endif
- viov->lease = st2lease(hdl->st);
hdl->st_off = 0;
hdl->st = next;
}
@@ -609,8 +607,6 @@ sml_ai_return(struct worker *wrk, vai_hdl vhdl, struct vscaret *scaret)
if (*p == VAI_LEASE_NORET)
continue;
CAST_OBJ_NOTNULL(st, lease2st(*p), STORAGE_MAGIC);
- if (st == hdl->last)
- continue;
VSCARET_ADD(todo, *p);
}
VSCARET_INIT(scaret, scaret->capacity);
@@ -804,7 +800,13 @@ sml_iterator(struct worker *wrk, struct objcore *oc,
uu = u;
if ((islast && nn < 0) || scaret->used == scaret->capacity - 1)
uu |= OBJ_ITER_FLUSH;
- r = func(priv, uu, vio->iov.iov_base, vio->iov.iov_len);
+
+ // null iov with the only purpose to return the resume ptr lease
+ // exception needed because assert(len > 0) in VDP_bytes()
+ if (vio->iov.iov_base == null_iov)
+ r = 0;
+ else
+ r = func(priv, uu, vio->iov.iov_base, vio->iov.iov_len);
if (r != 0)
break;
More information about the varnish-commit
mailing list