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

Re: [PATCH 1/2] packfile: fix race condition on unpack_entry()

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 28, 2020, 18:05 UTC
Message-ID
<xmqqpn65n5dr.fsf@gitster.c.googlers.com>
In-Reply-To
<42a7948f94cb57ebd9c37c3850b46b1ac9813ec6.1601311803.git.matheus.bernardino@usp.br>
Matheus Tavares <matheus.bernardino@usp.br> writes:
> To fix the race condition (and later segmentation fault), let's reorder
> the aforementioned steps so that the lock is not released between 1.
> and 3.

In other words, we hold the base in core only for ourselves without adding it to the base cache, apply the delta to produce the result and then place the base in the cache, and the reason why this change fixes the breakage is because the base we have locally and not in cache will not be seen by other people and will not be freed without our consent? Which does make sense.

But I was confused by the explanation "lock is not released". We do release the same lock when we call unpack_compressed_entry(), and reaquire it before the unpack_compressed_entry() returns. What the reordering achieves is to protect the base from getting evicted when the unlocking and relocing happens, no?

Thanks.
Previous: Matheus TavaresNext: Matheus Tavares
Message 6 of 12 in “RFC - concurrency causes segfault in git grep since 2.26.0”
  1. Phil HordSep 25, 2020
  2. Matheus TavaresSep 25, 2020
  3. Phil HordSep 25, 2020
  4. 0/2 Fix race condition and memory leak in delta base cacheMatheus Tavares, Sep 28, 2020
  5. 1/2 packfile: fix race condition on unpack_entry()Matheus Tavares, Sep 28, 2020
  6. Junio C HamanoSep 28, 2020
  7. 2/2 packfile: fix memory leak in add_delta_base_cache()Matheus Tavares, Sep 28, 2020
  8. Junio C HamanoSep 28, 2020
  9. 0/2 Fix race condition and memory leak in delta base cacheMatheus Tavares, Sep 29, 2020
  10. 1/2 packfile: fix race condition on unpack_entry()Matheus Tavares, Sep 29, 2020
  11. Phil HordOct 2, 2020
  12. 2/2 packfile: fix memory leak in add_delta_base_cache()Matheus Tavares, Sep 29, 2020

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.