On 06/23/2017 09:56 PM, Jeff King wrote: > On Fri, Jun 23, 2017 at 09:01:46AM +0200, Michael Haggerty wrote: > >> Change `repack_without_refs()` to expect the packed-refs lock to be >> held already, and not to release the lock before returning. Change the >> callers to deal with lock management. >> >> This change makes it possible for callers to hold the packed-refs lock >> for a longer span of time, a possibility that will eventually make it >> possible to fix some longstanding races. > > This is the sort of clue I was thinking about in my last email. :)
I'll try to be better about that in the future. And the reason to make `packed_ref_store` fulfill the `ref_store` interface is mostly indirect at the moment. It was important (to my sanity, if nothing else) to simplify the interface to the packed-refs code before rewriting its innards. Since the `ref_store` interface is familiar and is fairly close to what is needed, it seemed logical to use it. >> The only semantic change here is that `repack_without_refs()` used to >> forgot to release the lock in the `if (!removed)` exit path. That >> omission is now fixed. > > s/used to forgot/previously forgot/ or similar? Thanks; will fix. Michael
