Re-consider part of the concept, and refactor some code.

POSIX requires that a file description is referenced by mmap(2),
so the refcount does not reach 0 when the file is deleted while
it is still mapped. So POSIX-compliant systems can be expected to
retain the mapping after file deletion.

Unless the VMOD goes to the trouble of copying the file, the safe
way to update it is to delete it, then write a file with the same
name with the new contents. A check may happen to run between
deletion and the new write. But because of the property mentioned
above, this is not a real problem for memory-mapped files.

So the VMOD will work according to these rules:

- If a check cannot read a file due to ENOENT, it is not an error.
  (It is an error if the file does not exist on initial read.)
  In that case, the file is considered unchanged -- the current
  mapping continues as the cached file contents.

- This means that for the VMOD, a deleted file is *not* considered
  to be in error, provided it could be read initially (that is, it is
  already mapped).

- We set a flag when the file is deleted, so that the condition
  can be detected.

- Users are advised in the docs that the "delete, then write"
  procedure is the *only* method for updating the file that the
  VMOD supports. Other methods may work, some of the time, but
  you do it differently at your own risk.

Also, access(2) is not a reliable means to determine if the file
can be read and mapped, because if checks permissions against
the real UID/GID, whereas open(2) depends on effective UID/GID.

So:

- checks begin with open(2), then use fstat(2) for the stat check.

- Path searches are done by attempting open(2) with the filename
  on each directory in the path.

src/tests/errors.vtc

@@ -31,7 +31,7 @@ client c1 {
 	expect resp.http.Errmsg == "No error"
 } -run
 
-shell {rm -f ${tmpdir}/error}
+shell {chmod a-r ${tmpdir}/error}
 delay .1
 
 client c2 {
@@ -39,10 +39,13 @@ client c2 {
 	rxresp
 	expect resp.status == 200
 	expect resp.http.Error == "true"
-	expect resp.http.Errmsg ~ {^vmod file failure: vcl1\.rdr: cannot read info about}
+	expect resp.http.Errmsg ~ {^vmod file failure: vcl1\.rdr: cannot open}
 } -run
 
-shell {touch ${tmpdir}/error}
+shell {
+	rm -f ${tmpdir}/error
+	touch ${tmpdir}/error
+}
 delay .1
 
 client c1 -run

src/tests/get.vtc

@@ -92,10 +92,10 @@ logexpect l1 -v v1 -d 1 -g raw -q "Debug" {
 	expect * * Debug	{^vmod file: vcl3\.rdr: check for \S+ finished successfully at}
 } -run
 
-shell {rm -f ${tmpdir}/file}
+shell {chmod a-r ${tmpdir}/file}
 delay .1
 
-client c1 {
+client c2 {
 	txreq
 	rxresp
 	expect resp.status == 503
@@ -104,14 +104,29 @@ client c1 {
 
 logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
 	expect 0 * Begin	req
-	expect * = VCL_Error	{^rdr\.get\(\): vmod file failure: vcl3\.rdr: cannot read info about}
+	expect * = VCL_Error	{^rdr\.get\(\): vmod file failure: vcl3\.rdr: cannot open}
 	expect * = End
 } -run
 
 logexpect l1 -v v1 -d 1 -g raw -q "Error" {
-	expect * 0 Error	{^vmod file failure: vcl3\.rdr: cannot read info about}
+	expect * 0 Error	{^vmod file failure: vcl3\.rdr: cannot open}
 } -run
 
+shell {
+	rm -f ${tmpdir}/file
+	echo -n "quux baz bar foo" > ${tmpdir}/file
+}
+delay .1
+
+client c1 -run
+
+shell {rm -f ${tmpdir}/file}
+delay .1
+
+# The file is deleted, but the mapping is retained (until munmap), so
+# the client will still see the mapped contents.
+client c1 -run
+
 shell {echo -n "The quick brown fox jumps over the lazy dog." > ${tmpdir}/fox}
 
 varnish v1 -vcl {
@@ -136,26 +151,30 @@ client c1 {
 	txreq
 	rxresp
 	expect resp.status == 200
-	expect resp.http.Get ~ "fox"
+	expect resp.http.Get == "The quick brown fox jumps over the lazy dog."
 } -run
 
-shell {echo -n "twentieth century fox" > ${tmpdir}/fox}
+shell {
+	rm -f ${tmpdir}/fox
+	echo -n "twentieth century fox" > ${tmpdir}/fox
+}
 delay .1
 
-# Whether or not the client sees the change, although no update checks
-# are performed, depends on whether changes in MAP_PRIVATE mapped
-# files are reflected in the mapped page. According to mmap(2), this
-# is unspecified. The change is apparently seen on Linux. Check the
-# test log to see if it happened on the present platform.
-client c1 -run
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+	expect resp.http.Get == "The quick brown fox jumps over the lazy dog."
+} -run
 
-shell {rm -f ${tmpdir}/fox}
-delay .1
+varnish v1 -errvcl {cannot open} {
+	import ${vmod_file};
+	backend b { .host = "${bad_ip}"; }
 
-# On Linux at least, the mapping is retained after file deletion
-# (until munmap), so the client will still see the mapped contents.
-# XXX test on other platforms
-client c1 -run
+	sub vcl_init {
+		new rdr = file.reader("${tmpdir}/nosuchfile");
+	}
+}
 
 varnish v1 -errvcl {not a regular file} {
 	import ${vmod_file};

src/tests/info.vtc

@@ -57,7 +57,7 @@ client c1 {
 	expect resp.http.Next-Check <= 0.1
 } -run
 
-shell {rm -f ${tmpdir}/sz}
+shell {chmod a-r ${tmpdir}/sz}
 delay .1
 
 client c1 {
@@ -69,7 +69,7 @@ client c1 {
 
 logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
 	expect 0 * Begin	req
-	expect * = VCL_Error	{^rdr\.size\(\): vmod file failure: vcl1\.rdr: cannot read info about}
+	expect * = VCL_Error	{^rdr\.size\(\): vmod file failure: vcl1\.rdr: cannot open}
 	expect * = End
 } -run
 
@@ -94,12 +94,12 @@ varnish v1 -vcl {
 	}
 }
 
-shell {rm -f ${tmpdir}/mtime}
+shell {chmod a-r ${tmpdir}/mtime}
 delay .1
 
 logexpect l1 -v v1 -d 0 -g vxid -q "VCL_Error" {
 	expect 0 * Begin	req
-	expect * = VCL_Error	{^rdr\.mtime\(\): vmod file failure: vcl2\.rdr: cannot read info about}
+	expect * = VCL_Error	{^rdr\.mtime\(\): vmod file failure: vcl2\.rdr: cannot open}
 	expect * = End
 } -start
 
@@ -129,7 +129,7 @@ varnish v1 -vcl {
 	}
 }
 
-shell {rm -f ${tmpdir}/nxtchk}
+shell {chmod a-r ${tmpdir}/nxtchk}
 delay .1
 
 # next_check() is not affected by errors.

src/tests/synth.vtc

@@ -39,7 +39,7 @@ client c1 {
 	expect resp.body == "quux baz bar foo"
 } -run
 
-shell {rm -f ${tmpdir}/synth}
+shell {chmod a-r ${tmpdir}/synth}
 delay .1
 
 client c1 {
@@ -49,11 +49,14 @@ client c1 {
 
 logexpect l1 -v v1 -d 1 -g vxid -q "VCL_Error" {
 	expect 0 * Begin	req
-	expect * = VCL_Error	{^rdr\.synth\(\): vmod file failure: vcl1\.rdr: cannot read info about}
+	expect * = VCL_Error	{^rdr\.synth\(\): vmod file failure: vcl1\.rdr: cannot open}
 	expect * = End
 } -run
 
-shell {touch ${tmpdir}/synth}
+shell {
+	rm -f ${tmpdir}/synth
+	touch ${tmpdir}/synth
+}
 
 varnish v1 -vcl {
 	import ${vmod_file};

src/vmod_file.c

@@ -74,6 +74,7 @@ struct file_info {
 #define RDR_ERROR	(1 << 1)
 #define RDR_MAPPED	(1 << 2)
 #define RDR_TIMER_INIT	(1 << 3)
+#define RDR_DELETED	(1 << 4)
 
 struct VPFX(file_reader) {
 	unsigned		magic;
@@ -104,7 +105,7 @@ check(union sigval val)
 	struct VPFX(file_reader) *rdr;
 	struct file_info *info;
 	struct stat st;
-	int fd;
+	int fd = -1;
 	void *addr;
 	char timbuf[VTIM_FORMAT_SIZE];
 	int err;
@@ -127,7 +128,24 @@ check(union sigval val)
 	AZ(pthread_rwlock_wrlock(&rdr->lock));
 
 	errno = 0;
-	if (stat(info->path, &st) != 0) {
+	if ((fd = open(info->path, O_RDONLY)) < 0) {
+		if (errno == ENOENT && (rdr->flags & RDR_MAPPED) != 0) {
+			rdr->flags |= RDR_DELETED;
+			VSL(SLT_Debug, 0, "vmod file: %s.%s: %s is deleted but "
+			    "already mapped", rdr->vcl_name, rdr->obj_name,
+			    info->path);
+			goto out;
+		}
+		VERRMSG(rdr, "%s.%s: cannot open %s: %s", rdr->vcl_name,
+			rdr->obj_name, info->path, vstrerror(errno));
+		VSL(SLT_Error, 0, rdr->errbuf);
+		rdr->flags |= RDR_ERROR;
+		goto out;
+	}
+	rdr->flags &= ~RDR_DELETED;
+
+	errno = 0;
+	if (fstat(fd, &st) != 0) {
 		VERRMSG(rdr, "%s.%s: cannot read info about %s: %s",
 			rdr->vcl_name, rdr->obj_name, info->path,
 			vstrerror(errno));
@@ -170,15 +188,6 @@ check(union sigval val)
 	}
 	rdr->flags &= ~RDR_MAPPED;
 
-	errno = 0;
-	if ((fd = open(info->path, O_RDONLY)) < 0) {
-		VERRMSG(rdr, "%s.%s: cannot open %s: %s", rdr->vcl_name,
-			rdr->obj_name, info->path, vstrerror(errno));
-		VSL(SLT_Error, 0, rdr->errbuf);
-		rdr->flags |= RDR_ERROR;
-		goto out;
-	}
-
 	/*
 	 * By mapping the length st_size + 1, and due to the fact that
 	 * mmap(2) fills the region of the mapped page past the length of
@@ -194,10 +203,8 @@ check(union sigval val)
 			rdr->obj_name, info->path, vstrerror(errno));
 		VSL(SLT_Error, 0, rdr->errbuf);
 		rdr->flags |= RDR_ERROR;
-		closefd(&fd);
 		goto out;
 	}
-	closefd(&fd);
 	AN(addr);
 	rdr->flags |= RDR_MAPPED;
 
@@ -232,6 +239,9 @@ check(union sigval val)
  out:
 	AZ(pthread_rwlock_unlock(&rdr->lock));
 
+	if (fd != -1)
+		closefd(&fd);
+
 	if ((rdr->flags & RDR_ERROR) == 0 && info->log_checks) {
 		VTIM_format(VTIM_real(), timbuf);
 		VSL(SLT_Debug, 0, "vmod file: %s.%s: check for %s "
@@ -313,6 +323,7 @@ vmod_reader__init(VRT_CTX, struct VPFX(file_reader) **rdrp,
 	else {
 		struct vsb *search;
 		char *end, delim = ':';
+		int fd = -1;
 
 		AZ(info->path);
 		if (path == NULL || *path == '\0') {
@@ -333,12 +344,13 @@ vmod_reader__init(VRT_CTX, struct VPFX(file_reader) **rdrp,
 				VSB_putc(search, '/');
 			VSB_cat(search, name);
 			VSB_finish(search);
-			if (access(VSB_data(search), R_OK) != 0)
+			if ((fd = open(VSB_data(search), O_RDONLY)) < 0)
 				continue;
 
 			info->path = malloc(VSB_len(search) + 1);
 			if (info->path == NULL) {
 				VSB_destroy(&search);
+				closefd(&fd);
 				VFAIL(ctx, "new %s: allocating path", vcl_name);
 				return;
 			}
@@ -346,6 +358,8 @@ vmod_reader__init(VRT_CTX, struct VPFX(file_reader) **rdrp,
 			break;
 		}
 		VSB_destroy(&search);
+		if (fd != -1)
+			closefd(&fd);
 		if (info->path == NULL) {
 			VFAIL(ctx, "new %s: %s not found or not readable on "
 			      "path %s", vcl_name, name, path);
@@ -413,7 +427,7 @@ vmod_reader__init(VRT_CTX, struct VPFX(file_reader) **rdrp,
 	AZ(rdr->addr);
 	AZ(rdr->info->mtime.tv_sec);
 	AZ(rdr->info->mtime.tv_nsec);
-	AZ(rdr->flags & (RDR_INITIALIZED | RDR_ERROR));
+	AZ(rdr->flags & (RDR_INITIALIZED | RDR_ERROR | RDR_DELETED));
 	do {
 		VTIM_sleep(INIT_SLEEP_INTERVAL);
 	} while ((rdr->flags & (RDR_INITIALIZED | RDR_ERROR)) == 0);
@@ -425,6 +439,7 @@ vmod_reader__init(VRT_CTX, struct VPFX(file_reader) **rdrp,
 	}
 
 	AN(rdr->flags & RDR_MAPPED);
+	AZ(rdr->flags & RDR_DELETED);
 	AN(rdr->addr);
 	AN(rdr->info->mtime.tv_sec);
 	AN(rdr->info->mtime.tv_nsec);