Re: Memory leak with sparse-checkout

On Mon, Sep 20, 2021 at 09:45:14PM +0930, Calbabreaker wrote:

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.

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.

I'm adding Stolee to the CC to see if he thinks that would be sensible behavior or not.

Thanks, Taylor