tree.diffIndex: pointer being freed was not allocated #112
Description
Activity
can you paste the backtrace? you can get this using gdb from the coredump or running your code in gdb. if you know what's causing it, if you sprinkle gc() throughout your code (using node --expose-gc) you might be able to get this to reproduce more reliably.
Got it. Thanks. Sorry for the delay, this was next to impossible to accomplish in Mavericks -- had to get an Ubuntu VM running and it took 5 seconds.
Here is the backtrace, let me know how if I can help. It only happens when there are changes in the working directory or in the index. If the diffList is empty, the segfault never occurs.
Program received signal SIGSEGV, Segmentation fault. 0x00007ffff6c4acd3 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 (gdb) bt #0 0x00007ffff6c4acd3 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007ffff6c4b898 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x00000000005970eb in node::Buffer::Replace(char*, unsigned long, void (*)(char*, void*), void*) () #3 0x0000000000597179 in node::Buffer::~Buffer() () #4 0x00000000005971e9 in node::Buffer::~Buffer() () #5 0x00000000005985f6 in node::ObjectWrap::WeakCallback(v8::Persistent<v8::Value>, void*) () #6 0x0000000000755fec in v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector) () #7 0x0000000000771a68 in v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::internal::GCTracer*) () #8 0x0000000000771fd8 in v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollector, char const*, char const*) () #9 0x0000000000875d42 in v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, char const*) () #10 0x000018ffabb06214 in ?? () #11 0x000018ffabb060e1 in ?? () #12 0x00007fffffffa4f0 in ?? () #13 0x00007fffffffa548 in ?? () #14 0x000018ffabe6d6dd in ?? () #15 0x0000346482a5bc99 in ?? () #16 0x0000000300000000 in ?? () #17 0x00003e823bd87491 in ?? () #18 0x0000267fe4719da9 in ?? () #19 0x0000267fe4719da9 in ?? () #20 0x00003e823bdffea1 in ?? () #21 0x00003e823bdef199 in ?? () #22 0x00007fffffffa5a0 in ?? () #23 0x000018ffabb156ce in ?? () #24 0x0000267fe4719949 in ?? () #25 0x000008214232c321 in ?? () #26 0x0000267fe4719d71 in ?? () #27 0x0000267fe4719d91 in ?? () #28 0x0000267fe4719d91 in ?? () #29 0x0000000300000000 in ?? () #30 0x000018ffabb154c1 in ?? () #31 0x0000000700000000 in ?? () #32 0x00000ef93e7bd559 in ?? () #33 0x00007fffffffa5e8 in ?? () #34 0x000018ffabe6d555 in ?? () #35 0x0000267fe4719949 in ?? ()
Thanks
Well, it's somewhat narrowed down in that it's in Buffer, but I'm not entirely sure where it's coming from. If we can narrow down what causes the problem, that would be easier.
For example, does just doing the diff lead the segfault? Or do you have to look at the patch? And if you look at the patch, what part of it?
After some googling, this may depend on the version of node.js. Can you let me know which version you're using ?
Sure. I'm using v0.10.22.
In my diff function I am actually calling a couple different diffs and merging them together, then iterating the patches. I will set up a few different tests that hit different points of the code to try to narrow it down. But intuitively, I think you may be right -- the patches seem to be the common denominator.
You're right -- it's in the patches. When calling the diff functions, if I don't touch diffList.patches() then the segfault does not occur. Will look into this further.
please narrow it down to one function if possible. It might be easiest to --expose-gc and call gc() in your code.
Well, I have a test where all I am doing is diffList.patches().length and it is segfaulting. It looks to me the only thing that function does is produce an array of ConvenientPatch objects. So, ConvenientPatch constructor?
It's not the constructor, but calling .patches() calls diffList.patch(i) where 0 < i < length. So presumably the bug is in the patch method https://github.com/nodegit/nodegit/blob/master/src/diff_list.cc#L179
I'm still unclear on what the actual bug is since this code doesn't have much to do with Buffer. Can you double check the backtrace again?
I have a theory. I'm going to assume the backtrace you pasted earlier is an unrelated issue.
Can you apply the following patch to your local checkout and rebuild the code with npm run-script gen && npm install && npm test:
diff --git a/v0.18.0.json b/v0.18.0.json index 5c800ca..5812783 100644 --- a/v0.18.0.json +++ b/v0.18.0.json @@ -3689,6 +3689,7 @@ "name": "delta_out", "jsName": "delta", "cType": "const git_diff_delta **", + "copy": "git_diff_delta_dup", "cppClassName": "GitDelta", "jsClassName": "Delta", "isReturn": true,I went ahead and added that line "copy": "git_diff_delta_dup" to "cFunctionName": "git_diff_get_patch" and it appears to have fixed the problem. Any insight into what was going on?
Thanks
Nodegit has C++ bindings to libgit2. Those bindings aren't hand-written, they're generated with a code generator that I wrote. The code-generator takes a config file (v0.18.0.json) that specifies how to generate the bindings. The single trickiest issue is mapping memory lifecycle issues across v8 -- a garbage collected runtime -- and libgit2, which requires manual memory management. In general, functions like git_diff_delta malloc objects and expect the caller to free them. By convention, that becomes a destructor on an object that is called when v8 garbage collects. This works fine, except in the small handful of cases where the caller is NOT responsible for freeing objects themselves; this usually happens when a method returns a nested field of a struct and that field is freed when the parent is freed (in this case the patch contains a delta; you're not supposed to free the delta: freeing the patch frees the delta). For these cases, the convention is to duplicate the object (e.g., using git_diff_delta_dup) so it can last longer than its parent object -- this is necessary if you start passing around the child field in javascript land but let the parent get garbage collected.
can you submit a pull request with this fix?
No problem.
I get what seems to be the same or very similar bug to this on the latest version of nodegit (which includes the above mentioned fix, so I guess it's not the same bug, but the error message is the same and it is triggered during garbage collection). Backtrace is as follows:
node(51658,0x7fff7c0c2310) malloc: *** error for object 0x105076f20: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug Process 51658 stopped * thread #1: tid = 0x30844f, 0x00007fff8f906bc0 libsystem_malloc.dylib`malloc_error_break, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 frame #0: 0x00007fff8f906bc0 libsystem_malloc.dylib`malloc_error_break libsystem_malloc.dylib`malloc_error_break: -> 0x7fff8f906bc0: pushq %rbp 0x7fff8f906bc1: movq %rsp, %rbp 0x7fff8f906bc4: nop 0x7fff8f906bc5: nopl (%rax) (lldb) bt * thread #1: tid = 0x30844f, 0x00007fff8f906bc0 libsystem_malloc.dylib`malloc_error_break, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00007fff8f906bc0 libsystem_malloc.dylib`malloc_error_break frame #1: 0x00007fff8f908028 libsystem_malloc.dylib`free + 324 frame #2: 0x00000001053b7e74 libgit2.0.dylib`git_index_clear(index=0x000000010507b130) + 148 at index.c:327 frame #3: 0x00000001053b7d95 libgit2.0.dylib`index_free(index=0x000000010507b130) + 21 at index.c:302 frame #4: 0x00000001053b7d74 libgit2.0.dylib`git_index_free(index=0x000000010507b130) + 100 at index.c:315 frame #5: 0x00000001053ef61b libgit2.0.dylib`drop_index(repo=0x0000000105057c60) + 59 at repository.c:67 frame #6: 0x00000001053ef555 libgit2.0.dylib`git_repository_free(repo=0x0000000105057c60) + 117 at repository.c:85 frame #7: 0x0000000100e88743 nodegit.node`~GitRepo(this=0x000000010507f9e0) + 29 at repo.cc:38 frame #8: 0x0000000100e886fd nodegit.node`~GitRepo(this=0x000000010507f9e0) + 15 at repo.cc:37 frame #9: 0x000000010001d0e2 node`node::ObjectWrap::WeakCallback(v8::Persistent<v8::Value>, void*) + 98 frame #10: 0x00000001001a3e0f node`v8::internal::GlobalHandles::Node::PostGarbageCollectionProcessing(v8::internal::Isolate*, v8::internal::GlobalHandles*) + 93 frame #11: 0x00000001001a3874 node`v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector) + 78 frame #12: 0x00000001001ab0be node`v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::internal::GCTracer*) + 1032 frame #13: 0x00000001001aab76 node`v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollector, char const*, char const*) + 332 frame #14: 0x00000001001aa96f node`v8::internal::Heap::CollectAllGarbage(int, char const*) + 95 frame #15: 0x000000010018ac11 node`v8::internal::Execution::HandleStackGuardInterrupt(v8::internal::Isolate*) + 139
Has anyone run into this issue that seems to be happening in tree.diffIndex? It happens sporadically -- it seems to occur during garbage collection. My app crashes with:
I am not sure how to get any more information beyond this. Tried searching for malloc_error_break to try to insert a breakpoint but I am not seeing it.