git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 11/25] files_ref_store: put the packed files lock directly in this struct

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
May 22, 2017, 14:17 UTC
Message-ID
<65189c542226105b83163559192fe647484328d6.1495460199.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1495460199.git.mhagger@alum.mit.edu>

Instead of using a global `lock_file` instance for the main "packed-refs" file and using a pointer in `files_ref_store` to keep track of whether it is locked, embed the `lock_file` instance directly in the `files_ref_store` struct and use the new `is_lock_file_locked()` function to keep track of whether it is locked. This keeps related data together and makes the main reference store less of a special case.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs/files-backend.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bfc555a417..1db40432af 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -63,16 +63,12 @@ struct files_ref_store {
 	struct packed_ref_cache *packed;
 
 	/*
-	 * Iff the packed-refs file associated with this instance is
-	 * currently locked for writing, this points at the associated
-	 * lock (which is owned by somebody else).
+	 * Lock used for the "packed-refs" file. Note that this (and
+	 * thus the enclosing `files_ref_store`) must not be freed.
 	 */
-	struct lock_file *packed_refs_lock;
+	struct lock_file packed_refs_lock;
 };
 
-/* Lock used for the main packed-refs file: */
-static struct lock_file packlock;
-
 /*
  * Increment the reference count of *packed_refs.
  */
@@ -102,7 +98,7 @@ static void clear_packed_ref_cache(struct files_ref_store *refs)
 	if (refs->packed) {
 		struct packed_ref_cache *packed_refs = refs->packed;
 
-		if (refs->packed_refs_lock)
+		if (is_lock_file_locked(&refs->packed_refs_lock))
 			die("BUG: packed-ref cache cleared while locked");
 		refs->packed = NULL;
 		release_packed_ref_cache(packed_refs);
@@ -394,7 +390,7 @@ static void add_packed_ref(struct files_ref_store *refs,
 {
 	struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs);
 
-	if (!refs->packed_refs_lock)
+	if (!is_lock_file_locked(&refs->packed_refs_lock))
 		die("BUG: packed refs not locked");
 	add_ref_entry(get_packed_ref_dir(packed_ref_cache),
 		      create_ref_entry(refname, oid, REF_ISPACKED, 1));
@@ -1288,7 +1284,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 	}
 
 	if (hold_lock_file_for_update_timeout(
-			    &packlock, files_packed_refs_path(refs),
+			    &refs->packed_refs_lock, files_packed_refs_path(refs),
 			    flags, timeout_value) < 0)
 		return -1;
 	/*
@@ -1298,7 +1294,6 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
 	 * the packed-refs file.
 	 */
 	packed_ref_cache = get_packed_ref_cache(refs);
-	refs->packed_refs_lock = &packlock;
 	/* Increment the reference count to prevent it from being freed: */
 	acquire_packed_ref_cache(packed_ref_cache);
 	return 0;
@@ -1321,10 +1316,10 @@ static int commit_packed_refs(struct files_ref_store *refs)
 
 	files_assert_main_repository(refs, "commit_packed_refs");
 
-	if (!refs->packed_refs_lock)
+	if (!is_lock_file_locked(&refs->packed_refs_lock))
 		die("BUG: packed-refs not locked");
 
-	out = fdopen_lock_file(refs->packed_refs_lock, "w");
+	out = fdopen_lock_file(&refs->packed_refs_lock, "w");
 	if (!out)
 		die_errno("unable to fdopen packed-refs descriptor");
 
@@ -1342,11 +1337,10 @@ static int commit_packed_refs(struct files_ref_store *refs)
 	if (ok != ITER_DONE)
 		die("error while iterating over references");
 
-	if (commit_lock_file(refs->packed_refs_lock)) {
+	if (commit_lock_file(&refs->packed_refs_lock)) {
 		save_errno = errno;
 		error = -1;
 	}
-	refs->packed_refs_lock = NULL;
 	release_packed_ref_cache(packed_ref_cache);
 	errno = save_errno;
 	return error;
@@ -1364,10 +1358,9 @@ static void rollback_packed_refs(struct files_ref_store *refs)
 
 	files_assert_main_repository(refs, "rollback_packed_refs");
 
-	if (!refs->packed_refs_lock)
+	if (!is_lock_file_locked(&refs->packed_refs_lock))
 		die("BUG: packed-refs not locked");
-	rollback_lock_file(refs->packed_refs_lock);
-	refs->packed_refs_lock = NULL;
+	rollback_lock_file(&refs->packed_refs_lock);
 	release_packed_ref_cache(packed_ref_cache);
 	clear_packed_ref_cache(refs);
 }

-- 
2.11.0
Previous: Michael HaggertyNext: Michael Haggerty
Message 12 of 31 in “Prepare to separate out a packed_ref_store”
  1. 00/25 Prepare to separate out a packed_ref_storeMichael Haggerty, May 22, 2017
  2. 01/25 t3600: clean up permissions test properlyMichael Haggerty, May 22, 2017
  3. 02/25 refs.h: clarify docstring for the ref_transaction_update()-related fnsMichael Haggerty, May 22, 2017
  4. 03/25 ref_iterator_begin_fn(): fix docstringMichael Haggerty, May 22, 2017
  5. 04/25 files-backend: use `die("BUG: ...")`, not `die("internal error: ...")`Michael Haggerty, May 22, 2017
  6. 05/25 prefix_ref_iterator: don't trim too muchMichael Haggerty, May 22, 2017
  7. 06/25 refs_ref_iterator_begin(): don't check prefixes redundantlyMichael Haggerty, May 22, 2017
  8. 07/25 refs: use `size_t` indexes when iterating over ref transaction updatesMichael Haggerty, May 22, 2017
  9. 08/25 ref_store: take a `msg` parameter when deleting referencesMichael Haggerty, May 22, 2017
  10. 10/25 files-backend: move `lock` member to `files_ref_store`Michael Haggerty, May 22, 2017
  11. 12/25 files_transaction_cleanup(): new helper functionMichael Haggerty, May 22, 2017
  12. 11/25 files_ref_store: put the packed files lock directly in this structMichael Haggerty, May 22, 2017
  13. 09/25 lockfile: add a new method, is_lock_file_locked()Michael Haggerty, May 22, 2017
  14. 14/25 ref_transaction_prepare(): new optional step for reference updatesMichael Haggerty, May 22, 2017
  15. 16/25 ref_update_reject_duplicates(): use `size_t` rather than `int`Michael Haggerty, May 22, 2017
  16. 15/25 ref_update_reject_duplicates(): expose function to whole refs moduleMichael Haggerty, May 22, 2017
  17. 17/25 ref_update_reject_duplicates(): add a sanity checkMichael Haggerty, May 22, 2017
  18. 19/25 get_packed_ref_cache(): assume "packed-refs" won't change while lockedMichael Haggerty, May 22, 2017
  19. 20/25 read_packed_refs(): do more of the work of reading packed refsMichael Haggerty, May 22, 2017
  20. 22/25 refs_ref_iterator_begin(): handle `GIT_REF_PARANOIA`Michael Haggerty, May 22, 2017
  21. 23/25 create_ref_entry(): remove `check_name` optionMichael Haggerty, May 22, 2017
  22. 24/25 ref-filter: limit traversal to prefixMichael Haggerty, May 22, 2017
  23. 25/25 cache_ref_iterator_begin(): avoid priming unneeded directoriesMichael Haggerty, May 22, 2017
  24. Jeff KingMay 23, 2017
  25. Michael HaggertyMay 24, 2017
  26. Junio C HamanoMay 24, 2017
  27. 21/25 read_packed_refs(): report unexpected fopen() failuresMichael Haggerty, May 22, 2017
  28. 13/25 ref_transaction_commit(): check for valid `transaction->state`Michael Haggerty, May 22, 2017
  29. 18/25 should_pack_ref(): new function, extracted from `files_pack_refs()`Michael Haggerty, May 22, 2017
  30. Stefan BellerMay 22, 2017
  31. Junio C HamanoMay 24, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.