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

Re: [PATCH 0/4] midx: avoid potential rename-while-mapped on Windows

From
Derrick Stolee <stolee@gmail.com>
Date
Oct 13, 2021, 13:29 UTC
Message-ID
<42a8d2ef-3a67-ca2c-4334-b79975d45da7@gmail.com>
In-Reply-To
<cover.1633729502.git.me@ttaylorr.com>
On 10/8/2021 5:46 PM, Taylor Blau wrote:
Show 23 quoted lines
> (This topic is based on tb/repack-write-midx).
> 
> While looking through some failures in t5319 when Git is compiled with
> SANITIZE=leak, I noticed a curious pattern that basically looks like:
> 
>     struct multi_pack_index *m = load_multi_pack_index(object_dir, 0);
>     /* ... */
>     write_midx_internal();
> 
> This can cause problems on platforms where a memory mapped file cannot be
> overridden via a rename, because load_multi_pack_index() builds a new MIDX from
> scratch each time it is called without adding that MIDX to the object store. But
> write_midx_internal() loads a MIDX from the object store, causing the same
> region to be mapped twice.
> 
> The details are fully spelled out in the second patch, but the gist is that
> while write_midx_internal() unmaps *its* copy of the MIDX, the caller's copy is
> still mapped in memory.
> 
> Strangely, I could not get this to produce a failure on Windows, even though I
> would have expected one. I asked around off-list, and it sounds like it's
> possible this behavior has changed, or Windows' mmap-equivalent may not lock
> files that fit within a single page.
That's a good possibility. Once everything is loaded into memory, there is no
need for a file handle.
 
> In any case, we let's side step the issue entirely by only interacting with
> MIDXs that can be found from the object store.

I think this is a good strategy, and the patch series goes above and beyond this one failure by making it easier to get the MIDX from a specific alternate.

Thanks, -Stolee

Previous: Ævar Arnfjörð Bjarmason
Message 8 of 8 in “midx: avoid potential rename-while-mapped on Windows”
  1. 0/4 midx: avoid potential rename-while-mapped on WindowsTaylor Blau, Oct 8, 2021
  2. 1/4 midx.c: extract MIDX lookup by object_dirTaylor Blau, Oct 8, 2021
  3. 2/4 midx.c: lookup MIDX by object directory during expireTaylor Blau, Oct 8, 2021
  4. Derrick StoleeOct 13, 2021
  5. 3/4 midx.c: lookup MIDX by object directory during repackTaylor Blau, Oct 8, 2021
  6. 4/4 midx.c: guard against commit_lock_file() failuresTaylor Blau, Oct 8, 2021
  7. Ævar Arnfjörð BjarmasonOct 9, 2021
  8. Derrick StoleeOct 13, 2021

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.