Skip to contentSkip to conversation

refs/files: prevent memory leak by freeing packed_ref_store

PATCH Superseded Sven Strickroth via GitGitGadget 4 participants 6 messages Aug 3, 2024 – Aug 5, 2024

lore
Share & tools
lore

Sven Strickroth via GitGitGadget

author
#0 lore raw

PATCH 1/1 refs/files: prevent memory leak by freeing packed_ref_store

From: Sven Strickroth <[email protected]>

This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).

Signed-off-by Sven Strickroth <[email protected]>

---
refs/files: prevent memory leak by freeing packed_ref_store

This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1757%2Fcsware%2Frefs-files-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1757/csware/refs-files-v1
Pull-Request: https://github.com/git/git/pull/1757

 refs/files-backend.c | 1 +
 1 file changed, 1 insertion(+)
refs/files-backend.c +1 -0
index aa52d9be7c7..11551de8f84 100644
@@ -157,6 +157,7 @@ static void files_ref_store_release(struct ref_store *ref_store)
 	free_ref_cache(refs->loose);
 	free(refs->gitcommondir);
 	ref_store_release(refs->packed_ref_store);
+	free(refs->packed_ref_store);
 }
 
 static void files_reflog_path(struct files_ref_store *refs,

base-commit: e559c4bf1a306cf5814418d318cc0fea070da3c7

Signature · 1 line
gitgitgadget
#1 lore raw
Quoted 3 lines · Sven Strickroth via GitGitGadget

From: Sven Strickroth <[email protected]>

This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).

The format of references should match git log --format=reference, which would be:

    64a6dd8ffc (refs: implement removal of ref storages, 2024-06-06)

refs/files-backend.c static void files_ref_store_release(struct ref_store *ref_store) · line 161

Quoted 10 lines
refs/files-backend.c +1 -0
index aa52d9be7c7..11551de8f84 100644
@@ -157,6 +157,7 @@ static void files_ref_store_release(struct ref_store *ref_store)
 	free_ref_cache(refs->loose);
 	free(refs->gitcommondir);
 	ref_store_release(refs->packed_ref_store);
+	free(refs->packed_ref_store);
 }

Makes sense. packed_ref_store_init() returns a newly-allocated ref store, and ref_store_release() only releases the store contents. Consequently, we have to manually free the store here.

That does highlight that packed_ref_store_init() is misnamed and really should be called packed_ref_store_new(), as it also allocates the structure itself. But that's a #leftoverbit for another day, I'd say.

Out of curiosity, did you hit this memory leak in some of our tests, or did you just happen to stumble over it by chance?

Thanks!

Patrick

#2 lore raw

Am 05.08.2024 um 10:36 schrieb Patrick Steinhardt:

Quoted 4 lines

That does highlight that packed_ref_store_init() is misnamed and really should be called packed_ref_store_new(), as it also allocates the structure itself. But that's a #leftoverbit for another day, I'd say.

This would also be true for ref for files_ref_store_init and reftable_be_init.

Quoted 2 lines

Out of curiosity, did you hit this memory leak in some of our tests, or did you just happen to stumble over it by chance?

I found this while working on TortoiseGit which also uses libgit internally.

Signature · 3 lines
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server
author
#3 lore raw

PATCH v2 1/1 refs/files: prevent memory leak by freeing packed_ref_store

In reply to Sven Strickroth via GitGitGadget

From: Sven Strickroth <[email protected]>

This complements 64a6dd8ffc (refs: implement removal of ref storages, 2024-06-06).

Signed-off-by Sven Strickroth <[email protected]>

---
refs/files: prevent memory leak by freeing packed_ref_store

This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1757%2Fcsware%2Frefs-files-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1757/csware/refs-files-v2
Pull-Request: https://github.com/git/git/pull/1757

Range-diff vs v1:

1: c68d0de3d58 ! 1: 96018e7257c refs/files: prevent memory leak by freeing packed_ref_store
@@ Metadata
## Commit message ##
refs/files: prevent memory leak by freeing packed_ref_store

- This complements "refs: implement removal of ref storages" (64a6dd8ffc2f).
+ This complements 64a6dd8ffc (refs: implement removal of ref storages,
+ 2024-06-06).

Signed-off-by Sven Strickroth <[email protected]>
 refs/files-backend.c | 1 +
 1 file changed, 1 insertion(+)
refs/files-backend.c +1 -0
index aa52d9be7c7..11551de8f84 100644
@@ -157,6 +157,7 @@ static void files_ref_store_release(struct ref_store *ref_store)
 	free_ref_cache(refs->loose);
 	free(refs->gitcommondir);
 	ref_store_release(refs->packed_ref_store);
+	free(refs->packed_ref_store);
 }
 
 static void files_reflog_path(struct files_ref_store *refs,

base-commit: e559c4bf1a306cf5814418d318cc0fea070da3c7

Signature · 1 line
gitgitgadget