From 77cdd5c60f499d78fbbaf707accb8a399ca097ea Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Thu, 20 Oct 2022 09:42:23 +0200 Subject: [PATCH 01/18] Fix memory leaks --- .../manual/revwalk/file_history_walk.cc | 2 + generate/templates/partials/async_function.cc | 45 +++++++++++++++++++ generate/templates/partials/sync_function.cc | 15 +++++-- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index 714d6f5db3..d5f64b8581 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -291,6 +291,8 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() if (fileHistoryEvent->type != GIT_DELTA_UNMODIFIED) { baton->out->push_back(fileHistoryEvent); + } else { + delete fileHistoryEvent; } git_commit_free(currentCommit); diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 6041c16f3f..d62cd0f210 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -178,6 +178,33 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() { baton->result = result; {%endif%} + + {%each args|argsInfo as arg %} + {%if not arg.isSelf %} + {%if not arg.payloadFor %} + {%if not arg.isCallbackFunction %} + {%if not arg.isStructType %} + {%if not arg.payloadFor %} + {%if not arg.isStructType %} + {%if not arg.isReturn %} + {%if not arg.isPayload %} + {%if arg.name %} + {%if arg.cppClassName == 'String'%} + free((void*)baton->{{ arg.name }}); + {%endif%} + {%if arg.cppClassName == 'Wrapper'%} + free((void*)baton->{{ arg.name }}); + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endeach%} } void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleErrorCallback() { @@ -201,6 +228,17 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleErrorCallback() { {%each args|argsInfo as arg %} {%if arg.shouldAlloc %} {%if not arg.isCppClassStringOrArray %} + {%if arg.cppClassName == "GitBuf" %} + {%else%} + {%if arg | isOid %} + if (baton->{{ arg.name}}NeedsFree) { + baton->{{ arg.name}}NeedsFree = false; + free((void*)baton->{{ arg.name }}); + } + {%else%} + free((void*)baton->{{ arg.name }}); + {%endif%} + {%endif%} {%elsif arg | isOid %} if (baton->{{ arg.name}}NeedsFree) { baton->{{ arg.name}}NeedsFree = false; @@ -346,6 +384,13 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { {%each args|argsInfo as arg %} {%if arg.shouldAlloc %} {%if not arg.isCppClassStringOrArray %} + {%if arg.cppClassName == "GitBuf" %} + {%else%} + {%if arg | isOid %} + {%else%} + free((void*)baton->{{ arg.name }}); + {%endif%} + {%endif%} {%elsif arg | isOid %} if (baton->{{ arg.name}}NeedsFree) { baton->{{ arg.name}}NeedsFree = false; diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index 8868bb5ca8..815348115d 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -95,9 +95,18 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { {%each args|argsInfo as arg %} {%if arg | isOid %} - if (info[{{ arg.jsArg }}]->IsString()) { - free((void *)from_{{ arg.name }}); - } + if (info[{{ arg.jsArg }}]->IsString()) { + free((void *)from_{{ arg.name }}); + } + {%else%} + {%if not arg.isReturn %} + {%if arg.cppClassName == 'String'%} + free((void *)from_{{ arg.name }}); + {%endif%} + {%if arg.cppClassName == 'Wrapper'%} + free((void *)from_{{ arg.name }}); + {%endif%} + {%endif%} {%endif%} {%endeach%} From 5926ba183a2e2dfbec00cfca87e890a0ac2805fe Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Thu, 20 Oct 2022 11:33:01 +0200 Subject: [PATCH 02/18] Fix git_diff_line leak --- generate/input/descriptor.json | 7 +++++++ generate/templates/manual/include/functions/free.h | 1 + generate/templates/manual/src/functions/free.cc | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index bf0efcd0a9..b871626f9e 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1448,6 +1448,13 @@ "diff_format_email_options": { "ignore": true }, + "diff_line": { + "dependencies": [ + "../include/functions/free.h" + ], + "selfFreeing": true, + "freeFunctionName": "git_diff_line_free" + }, "diff_perfdata": { "selfFreeing": true, "cDependencies": [ diff --git a/generate/templates/manual/include/functions/free.h b/generate/templates/manual/include/functions/free.h index 873417b985..a1c1e4e9a8 100644 --- a/generate/templates/manual/include/functions/free.h +++ b/generate/templates/manual/include/functions/free.h @@ -8,5 +8,6 @@ #define NODEGIT_FREE_FUNCTIONS void git_remote_head_free(git_remote_head *remote_head); +void git_diff_line_free(const git_diff_line *diff_line); #endif diff --git a/generate/templates/manual/src/functions/free.cc b/generate/templates/manual/src/functions/free.cc index a52a7016f7..3d2081b7a9 100644 --- a/generate/templates/manual/src/functions/free.cc +++ b/generate/templates/manual/src/functions/free.cc @@ -7,3 +7,8 @@ void git_remote_head_free(git_remote_head *remote_head) { free(remote_head->symref_target); free(remote_head); } + +void git_diff_line_free(const git_diff_line *diff_line) { + free((void*)(diff_line->content)); + free((void*) diff_line); +} From 2900bf186212efcce557e6b3b9862b95236b52c5 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Thu, 20 Oct 2022 15:44:24 +0200 Subject: [PATCH 03/18] Fix leak freeing PatchData --- generate/templates/manual/src/convenient_patch.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/generate/templates/manual/src/convenient_patch.cc b/generate/templates/manual/src/convenient_patch.cc index f60f53257b..d31853b9bf 100644 --- a/generate/templates/manual/src/convenient_patch.cc +++ b/generate/templates/manual/src/convenient_patch.cc @@ -27,8 +27,10 @@ void PatchDataFree(PatchData *patch) { free((void *)line->content); free((void *)line); } + delete hunk->lines; delete hunk; } + delete patch->hunks; delete patch; } From 6c4e32d684ed3fad8f9d437868de884f94d5217f Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Fri, 21 Oct 2022 18:55:01 +0200 Subject: [PATCH 04/18] Fix leak We need to free duplicated memory we are responsible for that we obtained from libgit2 because nodegit duplicates it again when calling the wrapper --- generate/templates/partials/async_function.cc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index d62cd0f210..41dae93b52 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -311,6 +311,19 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { Nan::Set(result, Nan::New("{{ _return.returnNameOrName }}").ToLocalChecked(), v8ConversionSlot); {%endif%} {%endeach%} + + {%each args|argsInfo as arg %} + {%if not arg.shouldAlloc %} + {%if arg.ownedByThis|and arg.dupFunction|and arg.freeFunctionName|and arg.isReturn|and arg.selfFreeing %} + // We need to free duplicated memory we are responsible for that we obtained from libgit2 because + // nodegit duplicates it again when calling the wrapper + if(baton->{{ arg.name }} != NULL) { + {{ arg.freeFunctionName }}(baton->{{ arg.name }}); + } + {%endif%} + {%endif%} + {%endeach%} + {%if .|returnsCount == 1 %} v8::Local result = v8ConversionSlot; {%endif%} From 5160975002bc1b7a9ea6461ccfb723885c781388 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 18:15:58 +0200 Subject: [PATCH 05/18] Fix leaks git str array --- .../manual/src/str_array_converter.cc | 6 ++-- generate/templates/partials/async_function.cc | 34 ++++++++++++------- .../templates/partials/convert_from_v8.cc | 4 +-- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/generate/templates/manual/src/str_array_converter.cc b/generate/templates/manual/src/str_array_converter.cc index 732f16cf26..eab9a9063b 100644 --- a/generate/templates/manual/src/str_array_converter.cc +++ b/generate/templates/manual/src/str_array_converter.cc @@ -25,11 +25,9 @@ git_strarray *StrArrayConverter::Convert(v8::Local val) { } git_strarray * StrArrayConverter::AllocStrArray(const size_t count) { - const size_t size = sizeof(git_strarray) + (sizeof(char*) * count); - uint8_t* memory = reinterpret_cast(malloc(size)); - git_strarray *result = reinterpret_cast(memory); + git_strarray *result = (git_strarray *)malloc(sizeof(git_strarray)); + result->strings = (char **)malloc(sizeof(char*) * count); result->count = count; - result->strings = reinterpret_cast(memory + sizeof(git_strarray)); return result; } diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 41dae93b52..30372d1741 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -182,19 +182,22 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() { {%each args|argsInfo as arg %} {%if not arg.isSelf %} {%if not arg.payloadFor %} - {%if not arg.isCallbackFunction %} + {%if arg.cppClassName == 'GitStrarray' %} + {%if not arg.isReturn %} + {{ arg.freeFunctionName }}((git_strarray*)baton->{{ arg.name }}); + free((void*)baton->{{ arg.name }}); + {%endif%} + {%elsif not arg.isCallbackFunction %} {%if not arg.isStructType %} - {%if not arg.payloadFor %} - {%if not arg.isStructType %} - {%if not arg.isReturn %} - {%if not arg.isPayload %} - {%if arg.name %} - {%if arg.cppClassName == 'String'%} - free((void*)baton->{{ arg.name }}); - {%endif%} - {%if arg.cppClassName == 'Wrapper'%} - free((void*)baton->{{ arg.name }}); - {%endif%} + {%if not arg.isStructType %} + {%if not arg.isReturn %} + {%if not arg.isPayload %} + {%if arg.name %} + {%if arg.cppClassName == 'String'%} + free((void*)baton->{{ arg.name }}); + {%endif%} + {%if arg.cppClassName == 'Wrapper'%} + free((void*)baton->{{ arg.name }}); {%endif%} {%endif%} {%endif%} @@ -313,6 +316,13 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { {%endeach%} {%each args|argsInfo as arg %} + {%if arg.shouldAlloc|and arg.isReturn|and arg.cppClassName == 'Array' %} + {%if arg.cType == 'git_strarray *' %} + // We need to free the strarray + {{ arg.freeFunctionName }}(baton->{{ arg.name }}); + free((void *)baton->{{ arg.name }}); + {%endif%} + {%endif%} {%if not arg.shouldAlloc %} {%if arg.ownedByThis|and arg.dupFunction|and arg.freeFunctionName|and arg.isReturn|and arg.selfFreeing %} // We need to free duplicated memory we are responsible for that we obtained from libgit2 because diff --git a/generate/templates/partials/convert_from_v8.cc b/generate/templates/partials/convert_from_v8.cc index 8b928e7f46..6b4b74459c 100644 --- a/generate/templates/partials/convert_from_v8.cc +++ b/generate/templates/partials/convert_from_v8.cc @@ -14,9 +14,9 @@ {% elsif cppClassName == 'GitBuf' %} {%-- Print nothing --%} {%else%} - if ((info.Length() - 1) > {{ jsArg }} && info[{{ jsArg }}]->Is{{ cppClassName|cppToV8 }}()) { - {%endif%} + if ((info.Length() - 1) > {{ jsArg }} && info[{{ jsArg }}]->Is{{ cppClassName|cppToV8 }}()) { {%endif%} + {%endif%} {%if cppClassName == 'String'%} Nan::Utf8String {{ name }}(Nan::To(info[{{ jsArg }}]).ToLocalChecked()); From 9eb357eda4b1cda94108fbdda62e527938d9398d Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 13:11:03 +0200 Subject: [PATCH 06/18] Fix leak strdup in oid --- generate/templates/partials/convert_from_v8.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/generate/templates/partials/convert_from_v8.cc b/generate/templates/partials/convert_from_v8.cc index 6b4b74459c..29db4bd072 100644 --- a/generate/templates/partials/convert_from_v8.cc +++ b/generate/templates/partials/convert_from_v8.cc @@ -58,8 +58,8 @@ if (arrayVal->IsString()) { // Try and parse in a string to a git_oid Nan::Utf8String oidString(Nan::To(arrayVal).ToLocalChecked()); - - if (git_oid_fromstr(&from_{{ name }}[i], (const char *) strdup(*oidString)) != GIT_OK) { + string str = string(*oidString); + if (git_oid_fromstr(&from_{{ name }}[i], str.c_str()) != GIT_OK) { if (git_error_last()) { return Nan::ThrowError(git_error_last()->message); } else { @@ -90,8 +90,8 @@ // Try and parse in a string to a git_oid Nan::Utf8String oidString(Nan::To(info[{{ jsArg }}]).ToLocalChecked()); git_oid *oidOut = (git_oid *)malloc(sizeof(git_oid)); - - if (git_oid_fromstr(oidOut, (const char *) strdup(*oidString)) != GIT_OK) { + string str = string(*oidString); + if (git_oid_fromstr(oidOut, str.c_str()) != GIT_OK) { free(oidOut); if (git_error_last()) { From 06b04c7d50ee7fe35f21a0cb0067ce5671abdfe1 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 18:39:54 +0200 Subject: [PATCH 07/18] Fix leak in GitPatch::ConvenientFromDiff --- .../templates/manual/patches/convenient_patches.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/generate/templates/manual/patches/convenient_patches.cc b/generate/templates/manual/patches/convenient_patches.cc index e183afb750..855789b32a 100644 --- a/generate/templates/manual/patches/convenient_patches.cc +++ b/generate/templates/manual/patches/convenient_patches.cc @@ -156,6 +156,7 @@ void GitPatch::ConvenientFromDiffWorker::HandleOKCallback() { } delete baton->out; + delete baton; Local argv[2] = { Nan::Null(), @@ -185,8 +186,6 @@ void GitPatch::ConvenientFromDiffWorker::HandleOKCallback() { } free((void *)baton->error); - - return; } if (baton->error_code < 0) { @@ -197,9 +196,15 @@ void GitPatch::ConvenientFromDiffWorker::HandleOKCallback() { err }; callback->Call(1, argv, async_resource); - - return; } + while (!baton->out->empty()) { + PatchDataFree(baton->out->back()); + baton->out->pop_back(); + } + + delete baton->out; + + delete baton; Nan::Call(*callback, 0, NULL); } From c8afbe6fb38f300f645758e66cff1cc2be77799a Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 18:45:58 +0200 Subject: [PATCH 08/18] Fix leak freeing array from javascript --- generate/templates/partials/async_function.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 30372d1741..594f085738 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -182,15 +182,15 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() { {%each args|argsInfo as arg %} {%if not arg.isSelf %} {%if not arg.payloadFor %} - {%if arg.cppClassName == 'GitStrarray' %} - {%if not arg.isReturn %} + {%if not arg.isReturn %} + {%if arg.cppClassName == 'GitStrarray' %} {{ arg.freeFunctionName }}((git_strarray*)baton->{{ arg.name }}); free((void*)baton->{{ arg.name }}); - {%endif%} - {%elsif not arg.isCallbackFunction %} - {%if not arg.isStructType %} + {%elsif arg.cppClassName == 'Array' %} + free((void*)baton->{{ arg.name }}); + {%elsif not arg.isCallbackFunction %} {%if not arg.isStructType %} - {%if not arg.isReturn %} + {%if not arg.isStructType %} {%if not arg.isPayload %} {%if arg.name %} {%if arg.cppClassName == 'String'%} From 54087a6a07bcaa103a7311549e0a8b42eba9f3a4 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 19:02:12 +0200 Subject: [PATCH 09/18] Fix double freeing --- generate/templates/partials/async_function.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 594f085738..1607d5f3c1 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -436,10 +436,6 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { {%each args|argsInfo as arg %} {%if arg.isCppClassStringOrArray %} - {%if arg.freeFunctionName %} - {%elsif not arg.isConst%} - free((void *)baton->{{ arg.name }}); - {%endif%} {%elsif arg | isOid %} if (baton->{{ arg.name}}NeedsFree) { baton->{{ arg.name}}NeedsFree = false; From 7d855333134ef4ddae259a5bb10815a9a5a3fcd2 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 19:07:31 +0200 Subject: [PATCH 10/18] Fix leak in GitRevwalk::FileHistoryWalk --- generate/templates/manual/revwalk/file_history_walk.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index d5f64b8581..ac25b8b3f2 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -472,6 +472,7 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() callback->Call(2, argv, async_resource); delete baton->out; + delete baton; return; } @@ -494,6 +495,7 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() } free((void *)baton->error); + delete baton; return; } @@ -505,6 +507,7 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() err }; callback->Call(1, argv, async_resource); + delete baton; return; } From 07a45932adf8b3d37843a8bcf0eeef487a99c0a6 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Tue, 25 Oct 2022 19:31:08 +0200 Subject: [PATCH 11/18] Fix leak on freeing git_index_entry --- generate/input/descriptor.json | 4 ++++ generate/templates/manual/include/functions/free.h | 1 + generate/templates/manual/src/functions/free.cc | 5 +++++ 3 files changed, 10 insertions(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index b871626f9e..536804bc52 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -2034,6 +2034,10 @@ } }, "index_entry": { + "dependencies": [ + "../include/functions/free.h" + ], + "freeFunctionName": "git_index_entry_free", "isReturnable": true, "hasConstructor": true, "ignoreInit": true diff --git a/generate/templates/manual/include/functions/free.h b/generate/templates/manual/include/functions/free.h index a1c1e4e9a8..08920111a5 100644 --- a/generate/templates/manual/include/functions/free.h +++ b/generate/templates/manual/include/functions/free.h @@ -9,5 +9,6 @@ void git_remote_head_free(git_remote_head *remote_head); void git_diff_line_free(const git_diff_line *diff_line); +void git_index_entry_free(git_index_entry *index_entry); #endif diff --git a/generate/templates/manual/src/functions/free.cc b/generate/templates/manual/src/functions/free.cc index 3d2081b7a9..cb8e1b8075 100644 --- a/generate/templates/manual/src/functions/free.cc +++ b/generate/templates/manual/src/functions/free.cc @@ -12,3 +12,8 @@ void git_diff_line_free(const git_diff_line *diff_line) { free((void*)(diff_line->content)); free((void*) diff_line); } + +void git_index_entry_free(git_index_entry *index_entry) { + free((void*)(index_entry->path)); + free((void*) index_entry); +} From 7b5c85b7e063cb4e305364a2cf7474bf9490a4b0 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 26 Oct 2022 15:38:07 +0200 Subject: [PATCH 12/18] Free baton on error processing javascript arguments We only free baton on some cases, but we should try to free all previously allocated memory. --- generate/templates/partials/async_function.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 1607d5f3c1..4b760fc2a9 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -64,7 +64,9 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { ); if (!conversionResult.result) { - delete[] baton->{{ arg.name }}; + // TODO free previously allocated memory + free(baton->{{ arg.name }}); + delete baton; return Nan::ThrowError(Nan::New(conversionResult.error).ToLocalChecked()); } @@ -77,6 +79,7 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { { auto conversionResult = Configurable{{ arg.cppClassName }}::fromJavascript(nodegitContext, info[{{ arg.jsArg }}]); if (!conversionResult.result) { + delete baton; return Nan::ThrowError(Nan::New(conversionResult.error).ToLocalChecked()); } From e3fd6fe3fdfe9601b7e688f00309448ea318b27e Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 26 Oct 2022 16:36:47 +0200 Subject: [PATCH 13/18] Fix leak when the out parameter is not self freeing --- generate/templates/partials/async_function.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 4b760fc2a9..9a03b35d0a 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -334,6 +334,13 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { {{ arg.freeFunctionName }}(baton->{{ arg.name }}); } {%endif%} + {%elsif arg.isReturn %} + {%if not arg.selfFreeing %} + {%if arg.cppClassName == "GitBuf" %} + {%else%} + free((void *)baton->{{ arg.name }}); + {%endif%} + {%endif%} {%endif%} {%endeach%} From 7a518396a67c6711e629ae9b09dd791067530e77 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 26 Oct 2022 16:53:47 +0200 Subject: [PATCH 14/18] Fix leak strdup getting oid from string --- generate/templates/manual/commit/extract_signature.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/generate/templates/manual/commit/extract_signature.cc b/generate/templates/manual/commit/extract_signature.cc index b42e8f1889..0f92ab181b 100644 --- a/generate/templates/manual/commit/extract_signature.cc +++ b/generate/templates/manual/commit/extract_signature.cc @@ -28,7 +28,8 @@ NAN_METHOD(GitCommit::ExtractSignature) if (info[1]->IsString()) { Nan::Utf8String oidString(Nan::To(info[1]).ToLocalChecked()); baton->commit_id = (git_oid *)malloc(sizeof(git_oid)); - if (git_oid_fromstr(baton->commit_id, (const char *)strdup(*oidString)) != GIT_OK) { + string str = string(*oidString); + if (git_oid_fromstr(baton->commit_id, str.c_str()) != GIT_OK) { free(baton->commit_id); if (git_error_last()) { From 535395f1d8a60b22ab3af479e2cba817762bfb2b Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 26 Oct 2022 19:18:43 +0200 Subject: [PATCH 15/18] Fix leaks in sync functions --- generate/templates/partials/sync_function.cc | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index 815348115d..0e106fefca 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -92,6 +92,46 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { } // lock master scope end {%endif%} + {%each args|argsInfo as arg %} + {%if not arg.isSelf %} + {%if not arg.payloadFor %} + {%if not arg.isReturn %} + {%if arg.cppClassName == 'GitStrarray' %} + {{ arg.freeFunctionName }}((git_strarray*)from_{{ arg.name }}); + free((void*)from_{{ arg.name }}); + {%elsif arg.cppClassName == 'Array' %} + free((void*)from_{{ arg.name }}); + {%endif%} + {%endif%} + {%endif%} + {%endif%} + {%endeach%} + + {%each args|argsInfo as arg %} + {%if arg.shouldAlloc|and arg.isReturn|and arg.cppClassName == 'Array' %} + {%if arg.cType == 'git_strarray *' %} + // We need to free the strarray + {{ arg.freeFunctionName }}(from_{{ arg.name }}); + free((void *)from_{{ arg.name }}); + {%endif%} + {%endif%} + {%if not arg.shouldAlloc %} + {%if arg.ownedByThis|and arg.dupFunction|and arg.freeFunctionName|and arg.isReturn|and arg.selfFreeing %} + // We need to free duplicated memory we are responsible for that we obtained from libgit2 because + // nodegit duplicates it again when calling the wrapper + if(from_{{ arg.name }} != NULL) { + {{ arg.freeFunctionName }}(from_{{ arg.name }}); + } + {%endif%} + {%elsif arg.isReturn %} + {%if not arg.selfFreeing %} + {%if arg.cppClassName == "GitBuf" %} + {%else%} + free((void *)from_{{ arg.name }}); + {%endif%} + {%endif%} + {%endif%} + {%endeach%} {%each args|argsInfo as arg %} {%if arg | isOid %} From ee48c709843223810fb5fee6807aaa54cdc15164 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Wed, 26 Oct 2022 20:31:24 +0200 Subject: [PATCH 16/18] Fix leaks in extract_signature template --- .../templates/manual/commit/extract_signature.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/generate/templates/manual/commit/extract_signature.cc b/generate/templates/manual/commit/extract_signature.cc index 0f92ab181b..05029b801e 100644 --- a/generate/templates/manual/commit/extract_signature.cc +++ b/generate/templates/manual/commit/extract_signature.cc @@ -28,6 +28,7 @@ NAN_METHOD(GitCommit::ExtractSignature) if (info[1]->IsString()) { Nan::Utf8String oidString(Nan::To(info[1]).ToLocalChecked()); baton->commit_id = (git_oid *)malloc(sizeof(git_oid)); + baton->commit_idNeedsFree = true; string str = string(*oidString); if (git_oid_fromstr(baton->commit_id, str.c_str()) != GIT_OK) { free(baton->commit_id); @@ -40,6 +41,7 @@ NAN_METHOD(GitCommit::ExtractSignature) } } else { baton->commit_id = Nan::ObjectWrap::Unwrap(Nan::To(info[1]).ToLocalChecked())->GetValue(); + baton->commit_idNeedsFree = false; } // baton->field @@ -97,6 +99,10 @@ void GitCommit::ExtractSignatureWorker::HandleErrorCallback() { git_buf_dispose(&baton->signature); git_buf_dispose(&baton->signed_data); + if (baton->commit_idNeedsFree) { + free(baton->commit_id); + } + free(baton->field); delete baton; @@ -155,9 +161,11 @@ void GitCommit::ExtractSignatureWorker::HandleOKCallback() git_buf_dispose(&baton->signature); git_buf_dispose(&baton->signed_data); - if (baton->field != NULL) { - free((void *)baton->field); + if (baton->commit_idNeedsFree) { + free(baton->commit_id); } + free(baton->field); + delete baton; } From d4981f04261f34b98e5d78ff1b04fb59b5137819 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Fri, 28 Oct 2022 18:44:36 +0200 Subject: [PATCH 17/18] Fix issues with index entries --- generate/input/descriptor.json | 10 ++++++++++ generate/templates/manual/include/functions/copy.h | 2 +- generate/templates/manual/include/functions/free.h | 2 +- generate/templates/manual/src/functions/copy.cc | 10 ++++++++++ generate/templates/manual/src/functions/free.cc | 3 ++- generate/templates/partials/field_accessors.cc | 1 + generate/templates/partials/sync_function.cc | 10 +--------- 7 files changed, 26 insertions(+), 12 deletions(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 536804bc52..af15fb5118 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1881,11 +1881,19 @@ "git_index_free": { "ignore": true }, + "git_index_get_byindex": { + "return": { + "ownedByThis": true + } + }, "git_index_get_bypath": { "args": { "stage": { "isOptional": true } + }, + "return": { + "ownedByThis": true } }, "git_index_new": { @@ -2035,9 +2043,11 @@ }, "index_entry": { "dependencies": [ + "../include/functions/copy.h", "../include/functions/free.h" ], "freeFunctionName": "git_index_entry_free", + "dupFunction": "git_index_entry_dup", "isReturnable": true, "hasConstructor": true, "ignoreInit": true diff --git a/generate/templates/manual/include/functions/copy.h b/generate/templates/manual/include/functions/copy.h index 9983e575af..75818bb00d 100644 --- a/generate/templates/manual/include/functions/copy.h +++ b/generate/templates/manual/include/functions/copy.h @@ -9,7 +9,6 @@ const git_error *git_error_dup(const git_error *arg); const git_oid *git_oid_dup(const git_oid *arg); -const git_index_entry *git_index_entry_dup(const git_index_entry *arg); const git_index_time *git_index_time_dup(const git_index_time *arg); const git_time *git_time_dup(const git_time *arg); const git_diff_delta *git_diff_delta_dup(const git_diff_delta *arg); @@ -19,5 +18,6 @@ git_remote_head *git_remote_head_dup(const git_remote_head *src); void git_time_dup(git_time **out, const git_time *arg); void git_transfer_progress_dup(git_transfer_progress **out, const git_transfer_progress *arg); +void git_index_entry_dup(git_index_entry **dest, const git_index_entry *src); #endif diff --git a/generate/templates/manual/include/functions/free.h b/generate/templates/manual/include/functions/free.h index 08920111a5..c7db4b7f63 100644 --- a/generate/templates/manual/include/functions/free.h +++ b/generate/templates/manual/include/functions/free.h @@ -9,6 +9,6 @@ void git_remote_head_free(git_remote_head *remote_head); void git_diff_line_free(const git_diff_line *diff_line); -void git_index_entry_free(git_index_entry *index_entry); +void git_index_entry_free(const git_index_entry *index_entry); #endif diff --git a/generate/templates/manual/src/functions/copy.cc b/generate/templates/manual/src/functions/copy.cc index 90327650b0..820122d344 100644 --- a/generate/templates/manual/src/functions/copy.cc +++ b/generate/templates/manual/src/functions/copy.cc @@ -4,6 +4,8 @@ #include "git2.h" #include "git2/diff.h" +#include + const git_error *git_error_dup(const git_error *arg) { git_error *result = (git_error *)malloc(sizeof(git_error)); result->klass = arg->klass; @@ -35,3 +37,11 @@ git_remote_head *git_remote_head_dup(const git_remote_head *src) { : NULL; return dest; } + +void git_index_entry_dup(git_index_entry **dest, const git_index_entry *src) { + *dest = (git_index_entry *)malloc(sizeof(git_index_entry)); + memcpy(*dest, src, sizeof(git_index_entry)); + (*dest)->path = src->path + ? strdup(src->path) + : NULL; +} diff --git a/generate/templates/manual/src/functions/free.cc b/generate/templates/manual/src/functions/free.cc index cb8e1b8075..318cd61d6e 100644 --- a/generate/templates/manual/src/functions/free.cc +++ b/generate/templates/manual/src/functions/free.cc @@ -1,6 +1,7 @@ #include #include "git2.h" +#include void git_remote_head_free(git_remote_head *remote_head) { free(remote_head->name); @@ -13,7 +14,7 @@ void git_diff_line_free(const git_diff_line *diff_line) { free((void*) diff_line); } -void git_index_entry_free(git_index_entry *index_entry) { +void git_index_entry_free(const git_index_entry *index_entry) { free((void*)(index_entry->path)); free((void*) index_entry); } diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index bcd5e5f870..c0f6c56744 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -55,6 +55,7 @@ {% elsif field.cppClassName == 'String' %} if (wrapper->GetValue()->{{ field.name }}) { + free((void*)wrapper->GetValue()->{{ field.name }}); } Nan::Utf8String str(value); diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index 0e106fefca..4429f57c58 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -115,15 +115,7 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { free((void *)from_{{ arg.name }}); {%endif%} {%endif%} - {%if not arg.shouldAlloc %} - {%if arg.ownedByThis|and arg.dupFunction|and arg.freeFunctionName|and arg.isReturn|and arg.selfFreeing %} - // We need to free duplicated memory we are responsible for that we obtained from libgit2 because - // nodegit duplicates it again when calling the wrapper - if(from_{{ arg.name }} != NULL) { - {{ arg.freeFunctionName }}(from_{{ arg.name }}); - } - {%endif%} - {%elsif arg.isReturn %} + {%if arg.shouldAlloc|and arg.isReturn %} {%if not arg.selfFreeing %} {%if arg.cppClassName == "GitBuf" %} {%else%} From 842196f5cc03934b63f262eecf13713274463906 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Fri, 4 Nov 2022 13:49:31 +0100 Subject: [PATCH 18/18] Unregister filters before clean the memory --- generate/templates/manual/include/cleanup_handle.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/generate/templates/manual/include/cleanup_handle.h b/generate/templates/manual/include/cleanup_handle.h index 5eca8cf70d..0dd00eb887 100644 --- a/generate/templates/manual/include/cleanup_handle.h +++ b/generate/templates/manual/include/cleanup_handle.h @@ -5,6 +5,12 @@ #include #include +extern "C" { + #include + #include +} + + namespace nodegit { class CleanupHandle { public: @@ -14,6 +20,12 @@ namespace nodegit { class FilterRegistryCleanupHandles : public CleanupHandle { public: + ~FilterRegistryCleanupHandles() { + for(std::map>::iterator iter = registeredFilters.begin(); iter != registeredFilters.end(); ++iter) { + std::string filtername = iter->first; + git_filter_unregister(filtername.c_str()); + } + } std::map> registeredFilters; }; }