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

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);
 	}
 }

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 */