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

[PATCH 3/3] create_symref: write reflog while holding lock

From
Jeff King <peff@peff.net>
Date
Dec 29, 2015, 05:57 UTC
Message-ID
<20151229055725.GC17012@sigill.intra.peff.net>
In-Reply-To
<20151229055558.GA12848@sigill.intra.peff.net>

We generally hold a lock on the matching ref while writing to its reflog; this prevents two simultaneous writers from clobbering each other's reflog lines (it does not even have to be two symref updates; because we don't hold the lock, we could race with somebody writing to the pointed-to ref via HEAD, for example).

We can fix this by writing the reflog before we commit the lockfile. This runs the risk of writing the reflog but failing the final rename(), but at least we now err on the same side as the rest of the ref code.

Noticed-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
---
 refs/files-backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3d1994d..180c837 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2850,12 +2850,13 @@ static int create_symref_locked(struct ref_lock *lock, const char *refname,
 		return error("unable to fdopen %s: %s",
 			     lock->lk->tempfile.filename.buf, strerror(errno));
 
+	update_symref_reflog(lock, refname, target, logmsg);
+
 	/* no error check; commit_ref will check ferror */
 	fprintf(lock->lk->tempfile.fp, "ref: %s\n", target);
 	if (commit_ref(lock) < 0)
 		return error("unable to write symref for %s: %s", refname,
 			     strerror(errno));
-	update_symref_reflog(lock, refname, target, logmsg);
 	return 0;
 }
 
-- 
2.7.0.rc2.368.g1cbb535
Previous: Jeff KingNext: Jeff King
Message 16 of 25 in “improve symbolic-ref robustness”
  1. 0/4 improve symbolic-ref robustnessJeff King, Dec 20, 2015
  2. 1/4 symbolic-ref: propagate error code from create_symref()Jeff King, Dec 20, 2015
  3. 2/4 t1401: test reflog creation for git-symbolic-refJeff King, Dec 20, 2015
  4. 3/4 create_symref: modernize variable namesJeff King, Dec 20, 2015
  5. Michael HaggertyDec 28, 2015
  6. Jeff KingDec 29, 2015
  7. 4/4 create_symref: use existing ref-lock codeJeff King, Dec 20, 2015
  8. Junio C HamanoDec 21, 2015
  9. Jeff KingDec 22, 2015
  10. Michael HaggertyDec 28, 2015
  11. Jeff KingDec 29, 2015
  12. Jeff KingDec 29, 2015
  13. 0/3 improve symbolic-ref robustnessJeff King, Dec 29, 2015
  14. 1/3 create_symref: modernize variable namesJeff King, Dec 29, 2015
  15. 2/3 create_symref: use existing ref-lock codeJeff King, Dec 29, 2015
  16. 3/3 create_symref: write reflog while holding lockJeff King, Dec 29, 2015
  17. 4/3 create_symref: drop support for writing symbolic linksJeff King, Dec 29, 2015
  18. Jeff KingDec 29, 2015
  19. Junio C HamanoDec 29, 2015
  20. Jeff KingDec 30, 2015
  21. Jeff KingDec 30, 2015
  22. Michael HaggertyDec 29, 2015
  23. Junio C HamanoDec 29, 2015
  24. Junio C HamanoDec 29, 2015
  25. Jeff KingDec 30, 2015

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.