"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
got patch: lock the worktree
To:
gameoftrees@openbsd.org
Date:
Wed, 14 Aug 2024 15:38:23 +0200

Download raw body.

Thread
noticed while working on the path_info() stuff.  we should definitely
lock exclusively the worktree since we may update the fileindex.
The way the fileindex is written (temp file + rename) is safe against
concurrent modifications, but could still mean that another concurrent
write operation could interfer with a patch operation.

it's not a new thing, i believe it's there since i wrote got patch :/

diff /home/op/w/got
commit - 70cea17ff7df1d1c70cefb074ffba2d8749ca4a3
path + /home/op/w/got
blob - b64451095294474244135e83c0947ddcc71fdb8a
file + include/got_worktree.h
--- include/got_worktree.h
+++ include/got_worktree.h
@@ -653,4 +653,5 @@ got_worktree_patch_schedule_rm(const char *, struct go
 
 /* Complete the patch operation. */
 const struct got_error *
-got_worktree_patch_complete(struct got_fileindex *, const char *);
+got_worktree_patch_complete(struct got_worktree *, struct got_fileindex *,
+    const char *);
blob - 91c3f6f39b56d17a00286c9df20fe296b864650c
file + lib/patch.c
--- lib/patch.c
+++ lib/patch.c
@@ -1138,9 +1138,8 @@ got_patch(int fd, struct got_worktree *worktree, struc
 	}
 
 done:
-	if (fileindex != NULL)
-		complete_err = got_worktree_patch_complete(fileindex,
-		    fileindex_path);
+	complete_err = got_worktree_patch_complete(worktree, fileindex,
+	    fileindex_path);
 	if (complete_err && err == NULL)
 		err = complete_err;
 	free(fileindex_path);
blob - 56a6662106eec9f8fe5d71909c8c9712a67c3eed
file + lib/worktree.c
--- lib/worktree.c
+++ lib/worktree.c
@@ -10148,6 +10148,12 @@ got_worktree_patch_prepare(struct got_fileindex **file
     char **fileindex_path, struct got_worktree *worktree,
     struct got_repository *repo)
 {
+	const struct got_error *err;
+
+	err = lock_worktree(worktree, LOCK_EX);
+	if (err)
+		return err;
+
 	return open_fileindex(fileindex, fileindex_path, worktree,
 	    got_repo_get_object_format(repo));
 }
@@ -10249,13 +10255,20 @@ got_worktree_patch_schedule_rm(const char *path, struc
 }
 
 const struct got_error *
-got_worktree_patch_complete(struct got_fileindex *fileindex,
+got_worktree_patch_complete(struct got_worktree *worktree,
+    struct got_fileindex *fileindex,
     const char *fileindex_path)
 {
-	const struct got_error *err = NULL;
+	const struct got_error *err = NULL, *unlock_err;
 
-	err = sync_fileindex(fileindex, fileindex_path);
-	got_fileindex_free(fileindex);
+	if (fileindex) {
+		err = sync_fileindex(fileindex, fileindex_path);
+		got_fileindex_free(fileindex);
+	}
 
+	unlock_err = lock_worktree(worktree, LOCK_UN);
+	if (unlock_err && err == NULL)
+		err = unlock_err;
+
 	return err;
 }