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

Re: Memory leak with sparse-checkout

From
Derrick Stolee <stolee@gmail.com>
Date
Sep 20, 2021, 16:29 UTC
Message-ID
<8a0ddd8e-b585-8f40-c4b1-0a51f11e6b84@gmail.com>
In-Reply-To
<YUiuWSXO1P3JwerH@nand.local>
On 9/20/2021 11:52 AM, Taylor Blau wrote:
Show 11 quoted lines
> On Mon, Sep 20, 2021 at 09:45:14PM +0930, Calbabreaker wrote:
>> What did you do before the bug happened? (Steps to reproduce your issue)
>>
>> This was ran:
>>
>> git clone https://github.com/Calbabreaker/piano --sparse
>> cd piano
>> git sparse-checkout add any_text
>> git checkout deploy-frontend
>> git sparse-checkout init --cone
>> git sparse-checkout add any_text
Thank you for pointing this out.

I'll point out that this was likely found because "--sparse" does not initialize cone mode patterns, but you might have expected it to. This will increase the priority of adding something like "--sparse=cone" to the 'git clone' options.

Show 39 quoted lines
> Thanks for the reproduction. An even simpler one may be (inside of any
> repository):
> 
>     git sparse-checkout init
>     git sparse-checkout add dir
>     git sparse-checkout init --cone
>     git sparse-checkout add dir
> 
> The problem occurs because we keep existing entries when adding to the
> sparse-checkout list, and cone-mode patterns do not mix with
> non cone-mode patterns.
> 
> So after the first init and "add dir", your sparse-checkout file looks
> like:
> 
>   /*
>   !/*/
>   dir
> 
> but then when we convert to cone-mode and try and add "dir" (which in
> cone-mode we'll convert to "/dir/"), we run into trouble when adding the
> existing "dir" entry. That's because add_patterns_cone_mode() calls
> insert_recursive_pattern() on every entry in the existing list,
> including "dir".
> 
> So when we call insert_recursive_pattern() with any pattern list and
> path containing "dir", we first insert "dir" into the list, and then:
> 
>   char *slash = strrchr(e->pattern, '/');
>   char *oldpattern = e->pattern;
> 
>   if (slash == e->pattern)
>     break;
>   // trim off a slash, repeat
> 
> except slash is NULL because "dir" doesn't contain a slash. And that
> explains the problem you're seeing, because (a) we'll stay in that while
> loop forever, and (b) because each iteration allocates memory to
> accommodate the new pattern, so we'll eventually run out of memory.
Yikes! Thanks for digging into the details.
Show 17 quoted lines
> The wrong thing to do would be to handle this case by changing the
> conditional to "if (!slash || slash == e->pattern)", because we can't
> blindly carry forward some patterns which look like cone-mode patterns,
> since together the list of sparse-checkout entries may not represent a
> cone.
> 
> (An example here is if we added /foo/bar/baz/* without the corresponding
> /foo/, !/foo/*, and so on).
> 
> So I think the problem really is that we need to drop existing patterns
> when re-initializing the sparse-checkout in cone mode. We could try to
> recognize that existing patterns may already constitute a cone (and/or
> create a cone that covers the existing patterns).
> 
> But I think the easiest thing (if a little unfriendly) would be to just
> drop them and start afresh when re-initializing the sparse-checkout in
> cone mode.

This isn't sufficient, as a user can modify their .git/info/sparse-checkout file whenever they want, so we should fix this bug regardless. We could add a "Your existing patterns are not in cone mode" error.

It might still be a good idea to let "git sparse-checkout init --cone" overwrite the sparse-checkout file _if the file is not already in cone mode_.

Thanks, -Stolee

Previous: Taylor BlauNext: Taylor Blau
Message 3 of 15 in “Memory leak with sparse-checkout”
  1. CalbabreakerSep 20, 2021
  2. Taylor BlauSep 20, 2021
  3. Derrick StoleeSep 20, 2021
  4. Taylor BlauSep 20, 2021
  5. Derrick StoleeSep 20, 2021
  6. Derrick StoleeSep 20, 2021
  7. Taylor BlauSep 20, 2021
  8. Derrick StoleeSep 20, 2021
  9. Taylor BlauSep 20, 2021
  10. Derrick StoleeSep 21, 2021
  11. Taylor BlauSep 21, 2021
  12. Derrick StoleeSep 21, 2021
  13. Taylor BlauSep 21, 2021
  14. Derrick StoleeSep 22, 2021
  15. Taylor BlauSep 22, 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.