Rework fcore locking

- move lock and unlock to tus_file.c
- clear the fcore pointer argument when the lock is not acquired

src/tus_file.c

@@ -483,6 +483,40 @@ tus_file_fini(struct tus_file_core *fcore)
 	REPLACE(fcore->filename, NULL);
 	FREE_OBJ(fcore);
 }
+
+/* try to lock the file, loosing the pointer to it if it failes */
+int
+tus_file_trylock(struct tus_file_core **fcorep)
+{
+	int err = EINVAL;
+	struct tus_file_core *fcore;
+
+	AN(fcorep);
+	fcore = *fcorep;
+	if (fcore == NULL)
+		return (err);
+
+	CHECK_OBJ(fcore, VMOD_TUS_FILE_CORE_MAGIC);
+	do {
+		err = pthread_mutex_trylock(&fcore->mtx);
+		if (err == 0)
+			return (err);
+	} while (err == EINTR);
+
+	*fcorep = NULL;
+	return (err);
+}
+
+/* keep a reference across requests, but done with it for this request */
+void
+tus_file_unlock(struct tus_file_core **fcorep)
+{
+	struct tus_file_core *fcore;
+
+	TAKE_OBJ_NOTNULL(fcore, fcorep, VMOD_TUS_FILE_CORE_MAGIC);
+	AZ(pthread_mutex_unlock(&fcore->mtx));
+}
+
 /* under tus_server mtx to protect tus_files */
 void
 tus_file_del(struct tus_file_core **fcorep)

src/tus_file.h

@@ -137,6 +137,8 @@ VSPLAY_PROTOTYPE(tus_files, tus_file_core, entry, tus_file_cmp)
 void tus_file_init(void);
 void tus_file_load(struct VPFX(tus_server) *);
 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 **);
 struct tus_file_core *tus_file_new(VRT_CTX, struct VPFX(tus_server) *,
     enum tus_f_type, const char *, const char *, const char *);

src/tus_request.c

@@ -246,7 +246,7 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 	struct concat_embryo embryo;
 	char *q;
 	enum met_e m;
-	int ct_ok, lock = EINVAL;
+	int ct_ok, lock;
 	uintmax_t cl, off = UINTMAX_MAX, len = UINTMAX_MAX;
 	enum tus_f_type type = TUS_SINGLE;
 	struct tus_concat *c;
@@ -326,24 +326,22 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 	if (fcore != NULL)
 		r->schemeauth = WS_Copy(ctx->ws, tussrv->schemeauth, -1);
 
-	if (fcore != NULL)
-		lock = pthread_mutex_trylock(&fcore->mtx);
+	lock = tus_file_trylock(&fcore);
 	AZ(pthread_mutex_unlock(&tussrv->mtx));
 
+	if (lock == EBUSY) {
+		AZ(fcore);
+		r->status = 423;
+		return (0);
+	}
+
 	if (fcore != NULL) {
+		assert(lock == 0);
 		fdisk = fcore->disk;
 		AN(fdisk);
 		type = fdisk->type;
-	}
-
-	// https://github.com/tus/tus-resumable-upload-protocol/issues/146
-	if (fcore == NULL) {
-		assert (lock == EINVAL);
-	} else if (lock == EBUSY) {
-		r->status = 423;
-		return (0);
 	} else {
-		assert (lock == 0);
+		assert(lock == EINVAL);
 	}
 
 	// from here on, we hold a lock on fcore (if != NULL), which we either
@@ -363,14 +361,14 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 	if (m == DELETE) {
 		if (fcore == NULL) {
 			r->status = 404;
-		} else {
-			AZ(pthread_mutex_lock(&tussrv->mtx));
-			tus_file_del(&fcore);
-			// unlocks fcore
-			AZ(pthread_mutex_unlock(&tussrv->mtx));
-			r->status = 204;
+			return (0);
 		}
-		return (0);
+
+		AZ(pthread_mutex_lock(&tussrv->mtx));
+		tus_file_del(&fcore);
+		AZ(pthread_mutex_unlock(&tussrv->mtx));
+		AZ(fcore);
+		r->status = 204;
 	}
 
 	AZ(r->fcore);
@@ -382,12 +380,13 @@ tus_request(VRT_CTX, struct VPFX(tus_server) *tussrv,
 		AN(fcore);
 		AN(fdisk);
 		if (fdisk->location_length > 0) {
-			r->fcore = fcore;
 			r->status = 301;	/* done file */
 		} else {
-			AZ(pthread_mutex_unlock(&fcore->mtx));
+			tus_file_unlock(&fcore);
+			AZ(fcore);
 			r->status = 400;
 		}
+		r->fcore = fcore;
 		return (0);
 	}
 

src/vmod_tus.c

@@ -312,8 +312,8 @@ tus_task_free(VRT_CTX, void *ptr)
 	CAST_OBJ_NOTNULL(r, ptr, VMOD_TUS_RESPONSE_MAGIC);
 	if (r->fcore == NULL)
 		return;
-	AZ(pthread_mutex_unlock(&r->fcore->mtx));
-	r->fcore = NULL;
+	tus_file_unlock(&r->fcore);
+	AZ(r->fcore);
 }
 
 static const struct vmod_priv_methods priv_task_methods[1] = {{