r4967 - in trunk/varnish-cache: bin/varnishtest/tests lib/libvcl

[phk at varnish-cache.org]

Author: phk
Date: 2010-06-17 00:15:27 +0200 (Thu, 17 Jun 2010)
New Revision: 4967

Added:
   trunk/varnish-cache/bin/varnishtest/tests/r00722.vtc
Modified:
   trunk/varnish-cache/lib/libvcl/vcc_backend.c
   trunk/varnish-cache/lib/libvcl/vcc_compile.c
   trunk/varnish-cache/lib/libvcl/vcc_xref.c
Log:
Fix a problem in director teardown at vcl.discard time:

We didn't create/destroy directors and backends in a consistent order,
and in some case we even destroyed directors more than once.

Always destroy in opposite order of creation (which follows VCL
source order).

Turn the bottom element of the array into (only) an indication
of which backend/director is the default.

Fixes:	#722



Added: trunk/varnish-cache/bin/varnishtest/tests/r00722.vtc
===================================================================
--- trunk/varnish-cache/bin/varnishtest/tests/r00722.vtc	                        (rev 0)
+++ trunk/varnish-cache/bin/varnishtest/tests/r00722.vtc	2010-06-16 22:15:27 UTC (rev 4967)
@@ -0,0 +1,31 @@
+# $Id$
+
+test "Director cleanup fails on vcl.discard"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend {
+	director foo random {
+		{ .backend = s1; .weight = 1; }
+		{ .backend = { .host = "${s1_addr}"; .port = "${s1_port}";} .weight =1; }
+		{ .backend = { .host = "${s1_addr}"; .port = "${s1_port}";} .weight =1; }
+	}
+	sub vcl_recv {
+		set req.backend = foo;
+	}
+} -start
+
+
+varnish v1 -vcl+backend { }
+
+varnish v1 -cliok "vcl.list"
+varnish v1 -cliok "vcl.discard vcl1"
+
+client c1 {
+	txreq
+	rxresp
+} -run
+

Modified: trunk/varnish-cache/lib/libvcl/vcc_backend.c
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_backend.c	2010-06-16 20:23:12 UTC (rev 4966)
+++ trunk/varnish-cache/lib/libvcl/vcc_backend.c	2010-06-16 22:15:27 UTC (rev 4967)
@@ -607,8 +607,6 @@
 
 		sprintf(vgcname, "%.*s_%d", PF(tl->t_dir), serial);
 
-		Ff(tl, 0, "\tVRT_fini_dir(cli, VGCDIR(_%.*s));\n",
-		    PF(tl->t_dir));
 		vcc_ParseHostDef(tl, serial, vgcname);
 		if (tl->err) {
 			vsb_printf(tl->sb,
@@ -672,6 +670,7 @@
 {
 	struct token *t_first;
 	struct dirlist const *dl;
+	int isfirst;
 
 	t_first = tl->t;
 	vcc_NextToken(tl);		/* ID: director | backend */
@@ -682,14 +681,12 @@
 	vcc_NextToken(tl);
 
 
+	isfirst = tl->ndirector;
 	if (vcc_IdIs(t_first, "backend")) {
 		tl->t_policy = t_first;
 		vcc_ParseSimpleDirector(tl);
 	} else {
-		Fh(tl, 1, "\n#define VGC_backend__%.*s %d\n",
-		    PF(tl->t_dir), tl->ndirector);
 		vcc_AddDef(tl, tl->t_dir, R_BACKEND);
-		tl->ndirector++;
 		ExpectErr(tl, ID);		/* ID: policy */
 		tl->t_policy = tl->t;
 		vcc_NextToken(tl);
@@ -708,9 +705,14 @@
 		dl->func(tl);
 		if (!tl->err)
 			SkipToken(tl, '}');
+		Fh(tl, 1, "\n#define VGC_backend__%.*s %d\n",
+		    PF(tl->t_dir), tl->ndirector);
+		tl->ndirector++;
 		Fi(tl, 0,
 		    "\tVRT_init_dir(cli, VCL_conf.director, \"%.*s\",\n",
 		    PF(tl->t_policy));
+		Ff(tl, 0, "\tVRT_fini_dir(cli, VGCDIR(_%.*s));\n",
+		    PF(tl->t_dir));
 		Fi(tl, 0, "\t    VGC_backend__%.*s, &vgc_dir_priv_%.*s);\n",
 		    PF(tl->t_dir), PF(tl->t_dir));
 
@@ -722,6 +724,18 @@
 		vcc_ErrWhere(tl, t_first);
 		return;
 	}
+
+	if (isfirst == 1) {
+		/*
+		 * If this is the first backend|director explicitly
+		 * defined, use it as default backend.
+		 */
+		Fi(tl, 0,
+		    "\tVCL_conf.director[0] = VCL_conf.director[%d];\n",
+		    tl->ndirector - 1);
+		vcc_AddRef(tl, tl->t_dir, R_BACKEND);
+	}
+
 	tl->t_policy = NULL;
 	tl->t_dir = NULL;
 }

Modified: trunk/varnish-cache/lib/libvcl/vcc_compile.c
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_compile.c	2010-06-16 20:23:12 UTC (rev 4966)
+++ trunk/varnish-cache/lib/libvcl/vcc_compile.c	2010-06-16 22:15:27 UTC (rev 4967)
@@ -469,6 +469,7 @@
 	VTAILQ_INIT(&tl->sources);
 
 	tl->nsources = 0;
+	tl->ndirector = 1;
 
 	/* General C code */
 	tl->fc = vsb_newauto();
@@ -580,7 +581,7 @@
 		return (vcc_DestroyTokenList(tl, NULL));
 
 	/* Check if we have any backends at all */
-	if (tl->ndirector == 0) {
+	if (tl->ndirector == 1) {
 		vsb_printf(tl->sb,
 		    "No backends or directors found in VCL program, "
 		    "at least one is necessary.\n");

Modified: trunk/varnish-cache/lib/libvcl/vcc_xref.c
===================================================================
--- trunk/varnish-cache/lib/libvcl/vcc_xref.c	2010-06-16 20:23:12 UTC (rev 4966)
+++ trunk/varnish-cache/lib/libvcl/vcc_xref.c	2010-06-16 22:15:27 UTC (rev 4967)
@@ -143,10 +143,6 @@
 	}
 	r->defcnt++;
 	r->name = t;
-
-	/* The first backend is the default and thus has an implicit ref */
-	if (type == R_BACKEND && tl->ndirector == 0)
-		r->refcnt++;
 }
 
 /*--------------------------------------------------------------------*/