[6.0] c9e52f943 Allow EXP_Remove() to be called before EXP_Insert()

[Reza Naghibi]

commit c9e52f9434f043f2537afdcc87bd3b6ac8874dd7
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 3f81e2981..2ee61f8bb 100644
--- a/bin/varnishd/cache/cache_expire.c
+++ b/bin/varnishd/cache/cache_expire.c
@@ -135,7 +135,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;
 }
 
 
@@ -151,7 +151,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);
 	}
 }
@@ -165,22 +172,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);
+	}
 }
 
 /*--------------------------------------------------------------------
@@ -213,7 +235,12 @@ EXP_Rearm(struct objcore *oc, double now, double ttl, double grace, double keep)
 
 	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 83160b064..3f85ea807 100644
--- a/include/tbl/oc_exp_flags.h
+++ b/include/tbl/oc_exp_flags.h
@@ -33,6 +33,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 */