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

Re: [PATCH] fetch-pack: clear alternate shallow when complete

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Feb 5, 2019, 16:26 UTC
Message-ID
<20190205162636.128901-1-jonathantanmy@google.com>
In-Reply-To
<CACsJy8CvVPr+OJ04aLGDeAS-o5__x1+3cKQNuq2+qba5gw5W+g@mail.gmail.com>
Show 17 quoted lines
> On Mon, Feb 4, 2019 at 7:06 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > When we write an alternate shallow file in update_shallow, we write it
> > into the lock file. The string stored in alternate_shallow_file is
> > copied from the lock file path, but it is freed the moment that the lock
> > file is closed, since we call strbuf_release to free that path.
> >
> > This used to work, since we did not invoke git index-pack more than
> > once, but now that we do, we reuse the freed memory. Ensure we reset the
> > value to NULL to avoid using freed memory. git index-pack will read the
> > repository's shallow file, which will have been updated with the correct
> > information.
> 
> It may be worth mentioning bd0b42aed3 (fetch-pack: do not take shallow
> lock unnecessarily - 2019-01-10). I believe this is the same problem
> and a full solution was suggested but not implemented in that commit.

For reference, the full solution is [1], linked from that commit's email [2]. (Looking back, I probably should have included all the information below the "---" in the commit message proper.) The full solution is more related to shallow locks, though, not alternate_shallow_file.

[1] https://public-inbox.org/git/20181218010811.143608-1-jonathantanmy@google.com/ [2] https://public-inbox.org/git/20190110193645.34080-1-jonathantanmy@google.com/

> The problem with dangling alternate_shallow_file is also from that
> commit.
You're right - thanks for noticing this.
Show 6 quoted lines
> When line_received is false at the end of
> receive_shallow_info(), we should clear alternate_shallow_file. I'm
> still debating myself whether we should clear alternate_shallow_file
> in receive_shallow_info() in addition to your changes (which is good
> hygiene anyway) to keep the setup steps of do_fetch_pack() and
> do_fetch_pack_v2() aligned.

Clearing alternate_shallow_file when line_received is false at the end of receive_shallow_info() sounds like a good idea to me.

Previous: Duy NguyenNext: brian m. carlson
Message 7 of 13 in “t5702 failing under ASan on master”
  1. brian m. carlsonJan 30, 2019
  2. Duy NguyenJan 30, 2019
  3. brian m. carlsonJan 30, 2019
  4. Junio C HamanoJan 31, 2019
  5. fetch-pack: clear alternate shallow when completebrian m. carlson, Feb 4, 2019
  6. Duy NguyenFeb 4, 2019
  7. Jonathan TanFeb 5, 2019
  8. brian m. carlsonFeb 6, 2019
  9. fetch-pack: clear alternate shallow when completebrian m. carlson, Feb 6, 2019
  10. Duy NguyenFeb 7, 2019
  11. Junio C HamanoFeb 7, 2019
  12. Junio C HamanoFeb 7, 2019
  13. brian m. carlsonFeb 7, 2019

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.