Restore previous VRT_AddDirector semantics

Upon failure, don't destroy the underlying director implementation. When
the deletion code was split in c671bb66
in order to be reused in 61529854, it
mistakenly included the director implementation.

Instead, it's really the caller's job to tear down the backend if it
couldn't be added. Since vbe_destroy operates on a director, it is split
in two to avoid reintroducing the previous "undo" code duplication.

Refs #3176

bin/varnishd/cache/cache_backend.c

@@ -391,12 +391,11 @@ vbe_dir_event(const struct director *d, enum vcl_event_e ev)
 
 /*---------------------------------------------------------------------*/
 
-static void v_matchproto_(vdi_destroy_f)
-vbe_destroy(const struct director *d)
+static void
+vbe_free(struct backend *be)
 {
-	struct backend *be;
 
-	CAST_OBJ_NOTNULL(be, d->priv, BACKEND_MAGIC);
+	CHECK_OBJ_NOTNULL(be, BACKEND_MAGIC);
 
 	if (be->probe != NULL)
 		VBP_Remove(be);
@@ -421,6 +420,15 @@ vbe_destroy(const struct director *d)
 	FREE_OBJ(be);
 }
 
+static void v_matchproto_(vdi_destroy_f)
+vbe_destroy(const struct director *d)
+{
+	struct backend *be;
+
+	CAST_OBJ_NOTNULL(be, d->priv, BACKEND_MAGIC);
+	vbe_free(be);
+}
+
 /*--------------------------------------------------------------------*/
 
 static void
@@ -550,7 +558,6 @@ VCL_BACKEND
 VRT_new_backend_clustered(VRT_CTX, struct vsmw_cluster *vc,
     const struct vrt_backend *vrt)
 {
-	VCL_BACKEND d;
 	struct backend *be;
 	struct vcl *vcl;
 	const struct vrt_backend_probe *vbp;
@@ -607,18 +614,17 @@ VRT_new_backend_clustered(VRT_CTX, struct vsmw_cluster *vc,
 	VSC_C_main->n_backend++;
 	Lck_Unlock(&backends_mtx);
 
-	d = VRT_AddDirector(ctx, m, be, "%s", vrt->vcl_name);
-
-	/* NB: if VRT_AddDirector failed, be was already freed. */
-	if (d != NULL) {
-		be->director = d;
+	be->director = VRT_AddDirector(ctx, m, be, "%s", vrt->vcl_name);
 
+	if (be->director != NULL) {
 		/* for cold VCL, update initial director state */
 		if (be->probe != NULL && ! vcl->temp->is_warm)
 			VBP_Update_Backend(be->probe);
+		return (be->director);
 	}
 
-	return (d);
+	vbe_free(be);
+	return (NULL);
 }
 
 VCL_BACKEND

bin/varnishd/cache/cache_vrt_vcl.c

@@ -41,8 +41,6 @@
 #include "cache_director.h"
 #include "cache_vcl.h"
 
-static void deldirector(struct vcldir *);
-
 /*--------------------------------------------------------------------*/
 
 const char *
@@ -142,6 +140,15 @@ VCL_Rel(struct vcl **vcc)
 
 /*--------------------------------------------------------------------*/
 
+static void
+vcldir_free(struct vcldir *vdir)
+{
+
+	free(vdir->cli_name);
+	FREE_OBJ(vdir->dir);
+	FREE_OBJ(vdir);
+}
+
 VCL_BACKEND
 VRT_AddDirector(VRT_CTX, const struct vdi_methods *m, void *priv,
     const char *fmt, ...)
@@ -159,6 +166,10 @@ VRT_AddDirector(VRT_CTX, const struct vdi_methods *m, void *priv,
 	vcl = ctx->vcl;
 	CHECK_OBJ_NOTNULL(vcl, VCL_MAGIC);
 
+	// opportunistic, re-checked again under lock
+	if (vcl->temp == VCL_TEMP_COOLING && !DO_DEBUG(DBG_VTC_MODE))
+		return (NULL);
+
 	ALLOC_OBJ(vdir, VCLDIR_MAGIC);
 	AN(vdir);
 	ALLOC_OBJ(vdir->dir, DIRECTOR_MAGIC);
@@ -198,7 +209,7 @@ VRT_AddDirector(VRT_CTX, const struct vdi_methods *m, void *priv,
 	Lck_Unlock(&vcl_mtx);
 
 	if (temp == VCL_TEMP_COOLING) {
-		deldirector(vdir);
+		vcldir_free(vdir);
 		return (NULL);
 	}
 	if (!temp->is_warm && temp != VCL_TEMP_INIT)
@@ -207,16 +218,6 @@ VRT_AddDirector(VRT_CTX, const struct vdi_methods *m, void *priv,
 	return (vdir->dir);
 }
 
-static void
-deldirector(struct vcldir *vdir)
-{
-	if(vdir->methods->destroy != NULL)
-		vdir->methods->destroy(vdir->dir);
-	free(vdir->cli_name);
-	FREE_OBJ(vdir->dir);
-	FREE_OBJ(vdir);
-}
-
 void
 VRT_DelDirector(VCL_BACKEND *bp)
 {
@@ -238,8 +239,10 @@ VRT_DelDirector(VCL_BACKEND *bp)
 
 	if (temp->is_warm)
 		VDI_Event(d, VCL_EVENT_COLD);
+	if(vdir->methods->destroy != NULL)
+		vdir->methods->destroy(d);
 	assert (d == vdir->dir);
-	deldirector(vdir);
+	vcldir_free(vdir);
 }
 
 void