On Thu, 21 Feb 2019 12:12:28 -0500 Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Thu, Feb 21, 2019 at 12:07 PM Phillip Wood <phillip.wood@xxxxxxxxxxxx> wrote: > > On 21/02/2019 13:50, Michal Suchánek wrote: > > >> On Tue, Feb 19, 2019 at 12:05 AM Michal Suchanek <msuchanek@xxxxxxx> wrote: > > > The problem is we don't forbid worktree names ending with ".lock". > > > Which means that if we start to forbid them now existing worktrees > > > might become inaccessible. > > > > I think it is also racy as the renaming breaks the use of mkdir erroring > > out if the directory already exists. One solution is to have a lock > > entry in $GIT_COMMON_DIR/worktree-locks and make sure the code that > > iterates over the entries in $GIT_COMMON_DIR/worktrees skips any that > > have a corresponding ignores in $GIT_COMMON_DIR/worktree-locks. If the > > worktree-locks/<dir> is created before worktree/<dir> then it should be > > race free (you will have to remove the lock if the real entry cannot be > > created and then increment the counter and try again). Entries could > > also be locked on removal to prevent a race there. > > I wonder, though, how much this helps or hinders the use-case which > prompted this patch series in the first place; to wit, creating > hundreds or thousands of worktrees. Doing so serially was too slow, so > the many "git worktree add" invocations were instead run in parallel > (which led to "discovery" of race conditions). Using a global worktree > lock would serialize worktree creation, thus slowing it down once > again. I created thousands of worktrees only for stress-testing. The real workload needs only a dozen of them. That still leads to hitting a race condition occasionally and automation failure. Creating a separate lock directory will probably work. The question is when do you need to take the lock. Before adding a worktree, sure. Before deleting it as well. The problem is that deleting a worktree successfully without creating some broken state needs to exclude processes that might add stuff in the worktree directory. How many operations then do *not* need to take the lock? Thanks Michal
