Re: [PATCH 1/2] shallow: free local object_array allocations

On Fri, Jan 09, 2026 at 05:21:45PM +0100, Samo Pogačnik wrote:

Hi Patrick,
thanks a lot for the reply.

On Tue, 2026-01-06 at 08:44 +0100, Patrick Steinhardt wrote:

quoted

On Tue, Dec 09, 2025 at 06:11:19PM +0000, Samo Pogačnik via GitGitGadget
wrote:

quoted

From: =?UTF-8?q?Samo=20Poga=C4=8Dnik?= <redacted>

The local object_array 'stack' in get_shallow_commits() function
does not free its dynamic elements before the function returns.
As a result elements remain allocated and their reference forgotten.

I think the elements themselves are actually fine. We have the following
loop:

	while (commit || i < heads->nr || stack.nr) {

So while the stack still has entries, we'll keep on iteration.
Furthermore, there is no `break` or early return in the loop, so we can
sure that we actually pop every single element from the array.

That being said, what we _don't_ do is to free the array itself. So I'm
mostly splitting hairs with how the commit message is phrased, the
change looks correct to me.

What I'm wondering though is why we never hit this memory leak in our
test suite. I guess the reason is simply that we ain't got enough test
coverage around shallow clones. Have you seen this leak in the wild? And
if so, can we add a test case that surfaces it?

Actually, the test I've added with the patch 2/2 does not pass without this
memory fix in linux-leaks and linux-reftable-leaks test runs.

In that case it would make sense to point out this detail in the commit
message to make it a bit easier for the reviewer. Thanks!

Patrick