Commit 056e7441d0bdfa8d8f7b240b34841190cf1864b4

Stefan Sperling 2018-03-11T11:39:48

use a dedicated file for the work tree lock

diff --git a/lib/got_worktree_priv.h b/lib/got_worktree_priv.h
index 7a45058..621a020 100644
--- a/lib/got_worktree_priv.h
+++ b/lib/got_worktree_priv.h
@@ -20,24 +20,23 @@ struct got_worktree {
 	char *path_prefix;
 
 	/*
-	 * File descriptor for the file index, open while a work tree is open.
-	 * This is used to read the file index and to write out a new file
-	 * index, and also for locking the entire work tree.
-	 * When a work tree is opened, a shared lock on the file index is
+	 * File descriptor for the lock file, open while a work tree is open.
+	 * When a work tree is opened, a shared lock on the lock file is
 	 * acquired with flock(2). This shared lock is held until the work
 	 * tree is closed, i.e. throughout the lifetime of any operation
 	 * which uses a work tree.
-	 * Before any modifications are made to the on-disk state of meta data,
-	 * tracked files, or directory tree structure, this shared lock must
-	 * be upgraded to an exclusive lock.
+	 * Before any modifications are made to the on-disk state of work
+	 * tree meta data, tracked files, or directory tree structure, this
+	 * shared lock must be upgraded to an exclusive lock.
 	 */
-	int fd_fileindex;
+	int lockfd;
 };
 
 #define GOT_WORKTREE_GOT_DIR		".got"
 #define GOT_WORKTREE_FILE_INDEX		"fileindex"
 #define GOT_WORKTREE_REPOSITORY		"repository"
 #define GOT_WORKTREE_PATH_PREFIX	"path-prefix"
+#define GOT_WORKTREE_LOCK		"lock"
 #define GOT_WORKTREE_FORMAT		"format"
 
 #define GOT_WORKTREE_FORMAT_VERSION	1
diff --git a/lib/worktree.c b/lib/worktree.c
index 04c8fc0..cdfcd00 100644
--- a/lib/worktree.c
+++ b/lib/worktree.c
@@ -163,6 +163,11 @@ got_worktree_init(const char *path, struct got_reference *head_ref,
 		goto done;
 	}
 
+	/* Create an empty lock file. */
+	err = create_meta_file(gotpath, GOT_WORKTREE_LOCK, NULL);
+	if (err)
+		goto done;
+
 	/* Create an empty file index. */
 	err = create_meta_file(gotpath, GOT_WORKTREE_FILE_INDEX, NULL);
 	if (err)
@@ -218,7 +223,7 @@ got_worktree_open(struct got_worktree **worktree, const char *path)
 	char *refstr = NULL;
 	char *path_repos = NULL;
 	char *formatstr = NULL;
-	char *path_fileindex = NULL;
+	char *path_lock = NULL;
 	int version, fd = -1;
 	const char *errstr;
 
@@ -230,19 +235,14 @@ got_worktree_open(struct got_worktree **worktree, const char *path)
 		goto done;
 	}
 
-	if (asprintf(&path_fileindex, "%s/%s", gotpath,
-	    GOT_WORKTREE_FILE_INDEX) == -1) {
+	if (asprintf(&path_lock, "%s/%s", gotpath, GOT_WORKTREE_LOCK) == -1) {
 		err = got_error(GOT_ERR_NO_MEM);
-		path_fileindex = NULL;
+		path_lock = NULL;
 		goto done;
 	}
 
-	fd = open(path_fileindex, O_RDWR | O_NOFOLLOW, GOT_DEFAULT_FILE_MODE);
+	fd = open(path_lock, O_RDWR | O_EXLOCK | O_NONBLOCK);
 	if (fd == -1) {
-		err = got_error_from_errno();
-		goto done;
-	}
-	if (flock(fd, LOCK_SH | LOCK_NB) == -1) {
 		err = (errno == EWOULDBLOCK ? got_error(GOT_ERR_WORKTREE_BUSY)
 		    : got_error_from_errno());
 		goto done;
@@ -267,7 +267,7 @@ got_worktree_open(struct got_worktree **worktree, const char *path)
 		err = got_error(GOT_ERR_NO_MEM);
 		goto done;
 	}
-	(*worktree)->fd_fileindex = -1;
+	(*worktree)->lockfd = -1;
 
 	(*worktree)->path_worktree_root = strdup(path);
 	if ((*worktree)->path_worktree_root == NULL) {
@@ -284,7 +284,7 @@ got_worktree_open(struct got_worktree **worktree, const char *path)
 
 done:
 	free(gotpath);
-	free(path_fileindex);
+	free(path_lock);
 	if (err) {
 		if (fd != -1)
 			close(fd);
@@ -292,7 +292,7 @@ done:
 			got_worktree_close(*worktree);
 		*worktree = NULL;
 	} else
-		(*worktree)->fd_fileindex = fd;
+		(*worktree)->lockfd = fd;
 
 	return err;
 }
@@ -303,8 +303,8 @@ got_worktree_close(struct got_worktree *worktree)
 	free(worktree->path_worktree_root);
 	free(worktree->path_repo);
 	free(worktree->path_prefix);
-	if (worktree->fd_fileindex != -1)
-		close(worktree->fd_fileindex);
+	if (worktree->lockfd != -1)
+		close(worktree->lockfd);
 	free(worktree);
 }
 
diff --git a/regress/worktree/worktree_test.c b/regress/worktree/worktree_test.c
index ec99a28..b6bb45f 100644
--- a/regress/worktree/worktree_test.c
+++ b/regress/worktree/worktree_test.c
@@ -84,6 +84,7 @@ remove_workdir(const char *worktree_path)
 	remove_meta_file(worktree_path, GOT_WORKTREE_FILE_INDEX);
 	remove_meta_file(worktree_path, GOT_WORKTREE_REPOSITORY);
 	remove_meta_file(worktree_path, GOT_WORKTREE_PATH_PREFIX);
+	remove_meta_file(worktree_path, GOT_WORKTREE_LOCK);
 	remove_meta_file(worktree_path, GOT_WORKTREE_FORMAT);
 	remove_got_dir(worktree_path);
 	rmdir(worktree_path);
@@ -133,6 +134,8 @@ worktree_init(const char *repo_path)
 	/* Ensure required files were created. */
 	if (!check_meta_file_exists(worktree_path, GOT_REF_HEAD))
 		goto done;
+	if (!check_meta_file_exists(worktree_path, GOT_WORKTREE_LOCK))
+		goto done;
 	if (!check_meta_file_exists(worktree_path, GOT_WORKTREE_FILE_INDEX))
 		goto done;
 	if (!check_meta_file_exists(worktree_path, GOT_WORKTREE_REPOSITORY))
@@ -214,6 +217,14 @@ worktree_init_exists(const char *repo_path)
 	unlink(path);
 	free(path);
 
+	if (!obstruct_meta_file(&path, worktree_path, GOT_WORKTREE_LOCK))
+		goto done;
+	err = got_worktree_init(worktree_path, head_ref, "/", repo);
+	if (err != NULL && err->code == GOT_ERR_ERRNO && errno == EEXIST)
+		ok++;
+	unlink(path);
+	free(path);
+
 	if (!obstruct_meta_file(&path, worktree_path, GOT_WORKTREE_FILE_INDEX))
 		goto done;
 	err = got_worktree_init(worktree_path, head_ref, "/", repo);
@@ -252,9 +263,9 @@ done:
 	if (repo)
 		got_repo_close(repo);
 	free(gotpath);
-	if (ok == 5)
+	if (ok == 6)
 		remove_workdir(worktree_path);
-	return (ok == 5);
+	return (ok == 6);
 }
 
 #define RUN_TEST(expr, name) \