[master] 611a48e34 Allow EXP_Remove() to be called before EXP_Insert()

[Dridi Boukelmoune]

commit 611a48e34610c6330f3c7291f2cb89e7cba8c032
Author: Martin Blix Grydeland <martin at 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 */