Re: File locking issues with fetching submodules in parallel

From: Stefan Beller <sbeller@google.com>
Date: Feb 20, 2018, 20:40 UTC
Message-ID: <CAGZ79kZXtE84nfO_nx+H61+2B9SU7ASD4r4JozwV=8HZ050SCA@mail.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1802201709570.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

On Tue, Feb 20, 2018 at 8:20 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 6 quoted lines

> Hi Stefan (and other submodule wizards),
>
> Igor Melnichenko reported a submodule problem in a Git for Windows ticket:
>
>         https://github.com/git-for-windows/git/issues/1469
>

Thanks for bringing this up. Is it better to discuss it here on at that issue?

Show 7 quoted lines

> Part of it is due to Windows' behavior where you cannot read the same file
> in one process while you write it in another process.
>
> The other part is how our submodule code works in parallel. In particular,
> we seem to write the `.git` file maybe even every time a submodule is
> fetched, unconditionally, not even testing whether the .git file is
> already there and contains the correct contents?

only when they need to be newly cloned (as that is when the submodule--helper clone is invoked). When fetching, we'd not touch the '.git' file IIRC.

Show 5 quoted lines

> For some reason, the bug reporter saw a "Permission denied" on the `.git`
> file when we try to write it (and I am pretty certain that I tracked it
> down correctly to the `connect_work_tree_and_git_dir()` function). The
> intermittent "Permission denied" error seems to suggest that *another*
> process is accessing this file while we are writing it.

The reporter also reports this coming out of "git submodule add", which is not parallelized, but rather in the messy state of migrating as much code into C from shell, trying to not introduce to many bugs. ;)

So for add there is no parallelism I am aware of, which makes me wonder how the race is coming in there.

I recall making the decision to unconditionally write out the '.git' file for other operations like "git reset --hard --recurse-submodules" or "git checkout --force --recurse-submodules", as there you are really asking for a clean slate. I missed the Windows FS semantics for that case, so I guess re-reading the file to make sure it is already correct would avoid issues on Windows there. (though I do not yet recall how the parallel stuff would bite us there, as the file is written as the very first thing)

Show 11 quoted lines

> It also seems that this problem becomes worse if the firewall is turned
> on, in which case a couple of network operations become a little slower
> (which I could imagine to be the factor making the problems more likely).
>
> A plausible workaround would be to write the `.git` file only when needed
> (which also would fix a bug on macOS/Linux, although the window is
> substantially smaller: the reader could potentially read a
> partially-written file).
>
> But maybe we are simply holding onto an open handle to the `.git` file too
> long?

That could very well be the case.

> I tried to put together a bit more information here:
>
> https://github.com/git-for-windows/git/issues/1469#issuecomment-366932746

Thanks!

> Do you think there is an easy solution for this? You're much deeper in the
> submodule code than I ever want to be...

I'll take a look for open handles, though we're already using our wrapper functions like 'write_file', which is supposed to close the handle gracefully (and has undergone testing on Windows a lot as it is the standard write function)

Thanks, Stefan