Rework interaction between expiry and file deletions from elsewhere

There was an inherent race between the expiry thread and other file
deletions.

We solve this by making clear that expiry owns the reference. When
some user action requires deletion, expiry is set to "now" and the
file is removed from the index. We use the srvref as a marker
for index removal. After removal from the index, the file can
not be found any longer, so gaining an srvref is not possible.

When expiry is first to delete the file, the file is removed from
the index and the ref is lost.

src/tus_file.c

@@ -56,6 +56,7 @@ VSPLAY_GENERATE(tus_files, tus_file_core, entry, tus_file_cmp)
 static uintmax_t header_size;
 static unsigned tus_file_srvref(struct tus_file_core *);
 static unsigned tus_file_unref(struct tus_file_core *);
+static void tus_touch(const struct tus_file_core *, VCL_DURATION);
 
 
 /*
@@ -514,54 +515,79 @@ tus_file_unlock(struct tus_file_core **fcorep)
 	AZ(pthread_mutex_unlock(&fcore->mtx));
 }
 
-/* under borh fcore and server mtx */
-static inline void
-del_ref_transfer(struct tus_file_core *fcore)
+/* under both fcore and server mtx */
+static inline unsigned
+ref_transfer(struct tus_file_core *fcore)
 {
-	assert(fcore->srvref > 0);
-	fcore->ref += fcore->srvref;
+	unsigned r;
+
+	r = fcore->srvref;
 	fcore->srvref = 0;
+
+	fcore->ref += r;
 	assert(fcore->ref > 0);
+	return (r);
 }
 
 /* caller holds tus_server mtx */
 static void
-tus_file_del_locked(struct tus_file_core **fcorep)
+tus_file_del_shutdown(struct tus_file_core **fcorep)
 {
 	struct tus_file_core *fcore;
 	const struct tus_file_core *rm;
 
 	TAKE_OBJ_NOTNULL(fcore, fcorep, VMOD_TUS_FILE_CORE_MAGIC);
 
+	AN(ref_transfer(fcore));
 	rm = VSPLAY_REMOVE(tus_files, tus_server_files(fcore->server),
 	    fcore);
-	del_ref_transfer(fcore);
-
 	assert (rm == fcore);
 	tus_exp_delete(fcore);
 
 	(void) tus_file_unref_locked(fcore);
 }
 
-/* under tus_server mtx to protect tus_files */
-void
-tus_file_del(struct tus_file_core **fcorep)
+static inline void
+tus_file_remove(struct tus_file_core *fcore)
 {
 	struct VPFX(tus_server) *srv;
-	struct tus_file_core *fcore;
 	const struct tus_file_core *rm;
 
-	TAKE_OBJ_NOTNULL(fcore, fcorep, VMOD_TUS_FILE_CORE_MAGIC);
+	CHECK_OBJ_NOTNULL(fcore, VMOD_TUS_FILE_CORE_MAGIC);
+
 	srv = fcore->server;
 
 	tus_server_lock(srv);
-	rm = VSPLAY_REMOVE(tus_files, tus_server_files(srv), fcore);
-	del_ref_transfer(fcore);
+	if (ref_transfer(fcore)) {
+		rm = VSPLAY_REMOVE(tus_files, tus_server_files(srv), fcore);
+		assert (rm == fcore);
+	}
 	tus_server_unlock(srv);
+}
 
-	assert (rm == fcore);
-	tus_exp_delete(fcore);
+/* called from user under lock */
+void
+tus_file_del(struct tus_file_core **fcorep)
+{
+
+	AN(fcorep);
+	CHECK_OBJ_NOTNULL(*fcorep, VMOD_TUS_FILE_CORE_MAGIC);
+
+	tus_file_remove(*fcorep);
+	tus_touch(*fcorep, 0);
+	tus_file_unlock(fcorep);
+}
 
+/* called from expiry */
+void
+tus_file_exp(struct tus_file_core **fcorep)
+{
+	struct tus_file_core *fcore;
+
+	TAKE_OBJ_NOTNULL(fcore, fcorep, VMOD_TUS_FILE_CORE_MAGIC);
+
+	AZ(pthread_mutex_lock(&fcore->mtx));
+	tus_file_remove(fcore);
 	(void) tus_file_unref_locked(fcore);
 }
 
@@ -576,7 +602,7 @@ tus_file_shutdown(struct VPFX(tus_server) *srv)
 	VSPLAY_FOREACH(fcore, tus_files, files) {
 		REPLACE(fcore->filename, NULL); // prevent unlink
 		AZ(pthread_mutex_lock(&fcore->mtx));
-		tus_file_del_locked(&fcore);
+		tus_file_del_shutdown(&fcore);
 		AZ(fcore);
 	}
 	tus_server_unlock(srv);

src/tus_file.h

@@ -155,6 +155,7 @@ void tus_file_shutdown(struct VPFX(tus_server) *srv);
 int tus_file_trylock(struct tus_file_core **);
 void tus_file_unlock(struct tus_file_core **);
 void tus_file_del(struct tus_file_core **);
+void tus_file_exp(struct tus_file_core **);
 struct tus_file_core *tus_file_new(VRT_CTX, struct VPFX(tus_server) *,
     enum tus_f_type, const char *, const char *, const char *);
 struct tus_file_core *tus_file_lookup(struct VPFX(tus_server) *, const char *);

src/tus_file_exp.c

@@ -43,8 +43,6 @@
 #include "tus_file_exp.h"
 #include "tus_file_exp_setup.h"
 
-#define RETRY_SECONDS	10
-
 struct tus_exp {
 	unsigned	magic;
 #define TUS_EXP_MAGIC	0x105e8900
@@ -97,30 +95,20 @@ tus_exp_thread(void *p)
 			continue;
 		}
 
-		AZ(pthread_mutex_unlock(&e->mtx));
+		assert(fcore->exp_idx != VBH_NOIDX);
+		VBH_delete(e->heap, fcore->exp_idx);
 
+		AZ(pthread_mutex_unlock(&e->mtx));
 		AN(fcore);
-
-		while (fcore != NULL) {
-			struct tus_file_core *has_lock = fcore;
-
-			(void) tus_file_trylock(&has_lock);
-			if (has_lock != NULL)
-				tus_file_del(&fcore);
-
-			if (has_lock == NULL) {
-				fprintf(stderr, "tus expiry: fcore %p locked\n",
-				    fcore);
-				sleep(RETRY_SECONDS);
-			}
-		}
-
+		tus_file_exp(&fcore);
+		AZ(fcore);
 		AZ(pthread_mutex_lock(&e->mtx));
 	}
 	AZ(pthread_mutex_unlock(&e->mtx));
 	return (NULL);
 }
 
+/* only during shutdown, exp is already stopped */
 void
 tus_exp_delete(const struct tus_file_core *fcore)
 {
@@ -128,10 +116,10 @@ tus_exp_delete(const struct tus_file_core *fcore)
 
 	CHECK_OBJ_NOTNULL(e, TUS_EXP_MAGIC);
 
-	AZ(pthread_mutex_lock(&e->mtx));
+	assert(e->die == 1);
+	assert(e->thread == 0);
 	assert(fcore->exp_idx != VBH_NOIDX);
 	VBH_delete(e->heap, fcore->exp_idx);
-	AZ(pthread_mutex_unlock(&e->mtx));
 }
 
 void