[master] 68ac5a6 Some time ago I decided that it was more convenient if it were actually possible to locate the compiled VCL shared library based on the loaded VCL name, and removed the part of the subdirectory name which made it unique.

[Poul-Henning Kamp]

commit 68ac5a6690f37393771f4ecf923b00286aa95a0e
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Sat Jun 18 17:15:59 2016 +0000

    Some time ago I decided that it was more convenient if it were
    actually possible to locate the compiled VCL shared library based
    on the loaded VCL name, and removed the part of the subdirectory
    name which made it unique.
    
    Bad idea.
    
    Add a comment to make sure I don't get that Idea again.
    
    Fixes #1933

diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 055c841..41ae747 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -299,6 +299,11 @@ VCL_Open(const char *fn, struct vsb *msg)
 	AN(fn);
 	AN(msg);
 
+#ifdef RTLD_NOLOAD
+	/* Detect bogus caching by dlopen(3) */
+	dlh = dlopen(fn, RTLD_NOW | RTLD_NOLOAD);
+	AZ(dlh);
+#endif
 	dlh = dlopen(fn, RTLD_NOW | RTLD_LOCAL);
 	if (dlh == NULL) {
 		VSB_printf(msg, "Could not load compiled VCL.\n");
diff --git a/bin/varnishd/mgt/mgt_vcc.c b/bin/varnishd/mgt/mgt_vcc.c
index d7f3565..ca8ae94 100644
--- a/bin/varnishd/mgt/mgt_vcc.c
+++ b/bin/varnishd/mgt/mgt_vcc.c
@@ -47,6 +47,7 @@
 #include "vcli_serve.h"
 #include "vfil.h"
 #include "vsub.h"
+#include "vtim.h"
 
 struct vcc_priv {
 	unsigned	magic;
@@ -272,7 +273,39 @@ mgt_VccCompile(struct cli *cli, const char *vclname, const char *vclsrc,
 	vp.vclsrc = vclsrc;
 	vp.vclsrcfile = vclsrcfile;
 
-	VSB_printf(sb, "vcl_%s", vclname);
+	/*
+	 * The subdirectory must have a unique name to 100% certain evade
+	 * the refcounting semantics of dlopen(3).
+	 *
+	 * Bad implementations of dlopen(3) think the shlib you are opening
+	 * is the same, if the filename is the same as one already opened.
+	 *
+	 * Sensible implementations do a stat(2) and requires st_ino and
+	 * st_dev to also match.
+	 *
+	 * A correct implementation would run on filesystems which tickle
+	 * st_gen, and also insist that be the identical, before declaring
+	 * a match.
+	 *
+	 * Since no correct implementations are known to exist, we are subject
+	 * to really interesting races if you do something like:
+	 *
+	 *	(running on 'boot' vcl)
+	 *	vcl.load foo /foo.vcl
+	 *	vcl.use foo
+	 *	few/slow requests
+	 *	vcl.use boot
+	 *	vcl.discard foo
+	 *	vcl.load foo /foo.vcl	// dlopen(3) says "same-same"
+	 *	vcl.use foo
+	 *
+	 * Because discard of the first 'foo' lingers on non-zero reference
+	 * count, and when it finally runs, it trashes the second 'foo' because
+	 * dlopen(3) decided they were really the same thing.
+	 *
+	 * The Best way to reproduce ths is to have regexps in the VCL.
+	 */
+	VSB_printf(sb, "vcl_%s.%.9f", vclname, VTIM_real());
 	AZ(VSB_finish(sb));
 	vp.dir = strdup(VSB_data(sb));
 	AN(vp.dir);
diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index d9cd043..d084353 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -92,16 +92,25 @@ mgt_vcl_add(const char *name, const char *libfile, const char *state)
 static void
 mgt_vcl_del(struct vclprog *vp)
 {
-	char dn[256];
+	char *p;
 
 	VTAILQ_REMOVE(&vclhead, vp, list);
-	if (strcmp(vp->state, VCL_STATE_LABEL))
-		XXXAZ(unlink(vp->fname));
-	bprintf(dn, "vcl_%s", vp->name);
-	VJ_master(JAIL_MASTER_FILE);
-	(void)rmdir(dn);		// compiler droppings, eg gcov
-	VJ_master(JAIL_MASTER_LOW);
-	free(vp->fname);
+	if (strcmp(vp->state, VCL_STATE_LABEL)) {
+		AZ(unlink(vp->fname));
+		p = strrchr(vp->fname, '/');
+		AN(p);
+		*p = '\0';
+		VJ_master(JAIL_MASTER_FILE);
+		/*
+		 * This will fail if any files are dropped next to the library
+		 * without us knowing.  This happens for instance with GCOV.
+		 * Assume developers know how to clean up after themselves
+		 * (or alternatively:  How to run out of disk space).
+		 */
+		(void)rmdir(vp->fname);
+		VJ_master(JAIL_MASTER_LOW);
+		free(vp->fname);
+	}
 	free(vp->name);
 	free(vp);
 }