Skip to content

tree.diffIndex: pointer being freed was not allocated #112

Closed
Closed
tree.diffIndex: pointer being freed was not allocated#112

Description

@kmctown
kmctown
opened on Nov 12, 2013

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:

node(16403,0x7fff77c61310) malloc: 
*** error for object 0x100d05720: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

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.

Activity

  1. nkallen commented on Nov 15, 2013

    @nkallen
    nkallen
    on Nov 15, 2013
    Contributor

    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.

  2. kmctown commented on Nov 16, 2013

    @kmctown
    kmctown
    on Nov 16, 2013
    CollaboratorAuthor

    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

  3. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    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?

  4. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    After some googling, this may depend on the version of node.js. Can you let me know which version you're using ?

  5. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    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.

  6. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    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.

  7. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    please narrow it down to one function if possible. It might be easiest to --expose-gc and call gc() in your code.

  8. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    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?

  9. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    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?

  10. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    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,

  11. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    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?

  12. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    Thanks

  13. kmctown
    closed this as completedon Nov 18, 2013

  14. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    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.

  15. nkallen commented on Nov 18, 2013

    @nkallen
    nkallen
    on Nov 18, 2013
    Contributor

    can you submit a pull request with this fix?

  16. kmctown commented on Nov 18, 2013

    @kmctown
    kmctown
    on Nov 18, 2013
    CollaboratorAuthor

    No problem.

  17. kmctown commented on Nov 19, 2013

    @kmctown
    kmctown
    on Nov 19, 2013
    CollaboratorAuthor

    Pull request is submitted #113

  18. hugovincent commented on May 1, 2014

    @hugovincent
    hugovincent
    on May 1, 2014

    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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions