Mailing List Archive

[master] 611a48e34 Allow EXP_Remove() to be called before EXP_Insert()
commit 611a48e34610c6330f3c7291f2cb89e7cba8c032
Author: Martin Blix Grydeland <martin@varnish-software.com>
Date: Mon Mar 23 14:25:46 2020 +0100

Allow EXP_Remove() to be called before EXP_Insert()

Once HSH_Unbusy() has been called there is a possibility for
EXP_Remove() to be called before the fetch thread has had a chance to call
EXP_Insert(). By adding a OC_EF_NEW flag on the objects during
HSH_Unbusy(), that is removed again during EXP_Insert(), we can keep track
and clean up once EXP_Insert() is called by the inserting thread if
EXP_Remove() was called in the mean time.

This patch also removes the AZ(OC_F_DYING) in EXP_Insert(), as that is no
longer a requirement.

Fixes: #2999

diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c
index 12feff3da..c6bb36ef5 100644
--- a/bin/varnishd/cache/cache_expire.c
+++ b/bin/varnishd/cache/cache_expire.c
@@ -139,7 +139,7 @@ EXP_RefNewObjcore(struct objcore *oc)
AZ(oc->exp_flags);
assert(oc->refcnt >= 1);
oc->refcnt++;
- oc->exp_flags |= OC_EF_REFD;
+ oc->exp_flags |= OC_EF_REFD | OC_EF_NEW;
}


@@ -155,7 +155,14 @@ EXP_Remove(struct objcore *oc)
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
if (oc->exp_flags & OC_EF_REFD) {
Lck_Lock(&exphdl->mtx);
- exp_mail_it(oc, OC_EF_REMOVE);
+ if (oc->exp_flags & OC_EF_NEW) {
+ /* EXP_Insert has not been called for this object
+ * yet. Mark it for removal, and EXP_Insert will
+ * clean up once it is called. */
+ AZ(oc->exp_flags & OC_EF_POSTED);
+ oc->exp_flags |= OC_EF_REMOVE;
+ } else
+ exp_mail_it(oc, OC_EF_REMOVE);
Lck_Unlock(&exphdl->mtx);
}
}
@@ -169,22 +176,37 @@ EXP_Remove(struct objcore *oc)
void
EXP_Insert(struct worker *wrk, struct objcore *oc)
{
+ unsigned remove_race = 0;

CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);

+ AZ(oc->flags & OC_F_BUSY);
+
if (!(oc->exp_flags & OC_EF_REFD))
return;

assert(oc->refcnt >= 2);

- AZ(oc->flags & OC_F_DYING);
-
ObjSendEvent(wrk, oc, OEV_INSERT);
+
Lck_Lock(&exphdl->mtx);
- AZ(oc->exp_flags & (OC_EF_INSERT | OC_EF_MOVE));
- exp_mail_it(oc, OC_EF_INSERT | OC_EF_MOVE);
+ AN(oc->exp_flags & OC_EF_NEW);
+ oc->exp_flags &= ~OC_EF_NEW;
+ AZ(oc->exp_flags & (OC_EF_INSERT | OC_EF_MOVE | OC_EF_POSTED));
+ if (oc->exp_flags & OC_EF_REMOVE) {
+ /* We raced some other thread executing EXP_Remove */
+ remove_race = 1;
+ oc->exp_flags &= ~(OC_EF_REFD | OC_EF_REMOVE);
+ } else
+ exp_mail_it(oc, OC_EF_INSERT | OC_EF_MOVE);
Lck_Unlock(&exphdl->mtx);
+
+ if (remove_race) {
+ ObjSendEvent(wrk, oc, OEV_EXPIRE);
+ (void)HSH_DerefObjCore(wrk, &oc, 0);
+ AZ(oc);
+ }
}

/*--------------------------------------------------------------------
@@ -218,7 +240,12 @@ EXP_Rearm(struct objcore *oc, vtim_real now,

if (when < oc->t_origin || when < oc->timer_when) {
Lck_Lock(&exphdl->mtx);
- exp_mail_it(oc, OC_EF_MOVE);
+ if (oc->exp_flags & OC_EF_NEW) {
+ /* EXP_Insert has not been called yet, do nothing
+ * as the initial insert will execute the move
+ * operation. */
+ } else
+ exp_mail_it(oc, OC_EF_MOVE);
Lck_Unlock(&exphdl->mtx);
}
}
diff --git a/include/tbl/oc_exp_flags.h b/include/tbl/oc_exp_flags.h
index 926ab5d8e..ffa570006 100644
--- a/include/tbl/oc_exp_flags.h
+++ b/include/tbl/oc_exp_flags.h
@@ -35,6 +35,7 @@ OC_EXP_FLAG(REFD, refd, (1<<2))
OC_EXP_FLAG(MOVE, move, (1<<3))
OC_EXP_FLAG(INSERT, insert, (1<<4))
OC_EXP_FLAG(REMOVE, remove, (1<<5))
+OC_EXP_FLAG(NEW, new, (1<<6))
#undef OC_EXP_FLAG

/*lint -restore */
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit