Thread (25 messages) 25 messages, 4 authors, 2026-02-15

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

From: Samo Pogačnik <hidden>
Date: 2026-01-09 16:30:45

Hi Patrick,
thanks a lot for the reply.

On Tue, 2026-01-06 at 08:44 +0100, Patrick Steinhardt wrote:
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.

best regards, Samo

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help