[4.1] 3692c22 Don't trust dlopen refcounting.

[Lasse Karstensen]

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

    Don't trust dlopen refcounting.
    
    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
    
    Merge notes:
    * Removed label check, as that doesn't exist in 4.1
    * Rewrote commit title.
    
    Conflicts:
    	bin/varnishd/mgt/mgt_vcl.c

diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 00a27b6..b4ad437 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -295,6 +295,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 0aa3851..169c67a 100644
--- a/bin/varnishd/mgt/mgt_vcc.c
+++ b/bin/varnishd/mgt/mgt_vcc.c
@@ -47,6 +47,7 @@
 #include "vcli_priv.h"
 #include "vfil.h"
 #include "vsub.h"
+#include "vtim.h"
 
 struct vcc_priv {
 	unsigned	magic;
@@ -267,7 +268,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 858c5ec..21f84f7 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -91,13 +91,21 @@ 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);
-	XXXAZ(unlink(vp->fname));
-	bprintf(dn, "vcl_%s", vp->name);
+	AZ(unlink(vp->fname));
+	p = strrchr(vp->fname, '/');
+	AN(p);
+	*p = '\0';
 	VJ_master(JAIL_MASTER_FILE);
-	(void)rmdir(dn);		// compiler droppings, eg gcov
+	/*
+	 * 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);