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

Re: [PATCH 8/8] reflog_expire(): lock symbolic refs themselves, not their referent

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 13, 2015, 18:05 UTC
Message-ID
<xmqqoaoxoffe.fsf@gitster.dls.corp.google.com>
In-Reply-To
<54DE259C.4030001@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 11 quoted lines
> I also realized that Git's current policy is probably not tenable if one
> process is re-seating a symref at the same time as another process is
> expiring its reflog. The "git reflog expire HEAD" might grab
> "refs/heads/master.lock" then start rewriting "logs/HEAD". Meanwhile,
> "git checkout foo" would grab "HEAD.lock" (not being blocked by the
> "expire" process), then rewrite it to "ref: refs/heads/foo", then grab
> "refs/heads/foo.lock" before updating "logs/HEAD". So both processes
> could be writing "logs/HEAD" at the same time.
> ...
> Switching to holding only "HEAD.lock" while updating "logs/HEAD" is the
> right solution,...

We convinced ourselves that not locking the symref is wrong, but have we actually convinced us that not locking the underlying ref, as long as we have a lock on the symref, is safe?

To protect you, the holder of a lock on refs/remotes/origin/HEAD that happens to point at refs/remotes/origin/next, from somebody who is updating the underlying refs/remotes/origin/next directly without going through the symbolic ref (e.g. receive-pack), wouldn't the other party need to find any and all symbolic refs that point at the underlying ref and take locks on them?

As dereferencing a symbolic ref in the forward direction is much cheaper than in the reverse, and because you need to dereference it anyway, I wonder if we want the endgame to be "hold locks on both", not "just hold a lock on the symlink".

Previous: Stefan BellerNext: Stefan Beller
Message 36 of 46 in “Fix some problems with reflog expiration”
  1. 0/8 Fix some problems with reflog expirationMichael Haggerty, Feb 9, 2015
  2. 1/8 write_ref_sha1(): remove check for lock == NULLMichael Haggerty, Feb 9, 2015
  3. Stefan BellerFeb 10, 2015
  4. Jeff KingFeb 11, 2015
  5. 2/8 write_ref_sha1(): Move write elision test to callersMichael Haggerty, Feb 9, 2015
  6. Junio C HamanoFeb 12, 2015
  7. 3/8 lock_ref_sha1_basic(): do not set force_write for missing referencesMichael Haggerty, Feb 9, 2015
  8. Stefan BellerFeb 10, 2015
  9. Jeff KingFeb 11, 2015
  10. Stefan BellerFeb 11, 2015
  11. Michael HaggertyFeb 12, 2015
  12. 4/8 reflog: fix documentationMichael Haggerty, Feb 9, 2015
  13. Stefan BellerFeb 10, 2015
  14. 5/8 reflog: rearrange the manpageMichael Haggerty, Feb 9, 2015
  15. Stefan BellerFeb 10, 2015
  16. Michael HaggertyFeb 12, 2015
  17. Junio C HamanoFeb 12, 2015
  18. 6/8 reflog_expire(): ignore --updateref for symbolic referencesMichael Haggerty, Feb 9, 2015
  19. Stefan BellerFeb 11, 2015
  20. Michael HaggertyFeb 12, 2015
  21. Stefan BellerFeb 12, 2015
  22. Junio C HamanoFeb 12, 2015
  23. Jeff KingFeb 12, 2015
  24. Michael HaggertyFeb 13, 2015
  25. 7/8 reflog_expire(): never update a reference to null_sha1Michael Haggerty, Feb 9, 2015
  26. Eric SunshineFeb 9, 2015
  27. Michael HaggertyFeb 12, 2015
  28. 8/8 reflog_expire(): lock symbolic refs themselves, not their referentMichael Haggerty, Feb 9, 2015
  29. Stefan BellerFeb 11, 2015
  30. Junio C HamanoFeb 11, 2015
  31. Stefan BellerFeb 11, 2015
  32. Michael HaggertyFeb 12, 2015
  33. Stefan BellerFeb 12, 2015
  34. Michael HaggertyFeb 13, 2015
  35. Stefan BellerFeb 13, 2015
  36. Junio C HamanoFeb 13, 2015
  37. Stefan BellerFeb 13, 2015
  38. Junio C HamanoFeb 13, 2015
  39. Stefan BellerFeb 13, 2015
  40. Junio C HamanoFeb 13, 2015
  41. Michael HaggertyFeb 13, 2015
  42. Junio C HamanoFeb 13, 2015
  43. Michael HaggertyFeb 14, 2015
  44. Stefan BellerFeb 9, 2015
  45. refs.c: get rid of force_write flagStefan Beller, Feb 10, 2015
  46. Michael HaggertyFeb 12, 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.