RevWalk malloc error #239
Description
Activity
More info:
node.js: v0.10.32
os: OS X 10.9.4Thanks @probablycorey, I'm looking into this as I'm working in that area right now. Thanks for the test case!
Yeah, I'm getting that to give me the same error on my end. I'm getting about 100 or so commits until it happens though. Were you seeing it like almost immediately or after a lot of commits have been processed?
Thanks @johnhaley81! It consistently fails on the 94th commit for me. If I add console.log(commit.sha()) it fails at the 86th commit 1cfccb4.
From what I can tell, the more I log about the commit the quicker it crashes.
Ok, I narrowed it down to how we're handling things in the destructor in our generated C++ classes. This was super helpful! Thanks @probablycorey!
So this affects more than just RevWalk.
This is a behavior of how we pass back internal C objects on a V8 wrapper to JS. Right now we have a function call that creates a new V8 wrapper class that holds the C object that the user can then use as they wish. The problem is that each time that function is called we create a new wrapper. Once those variables go out of scope they are deleted. Unfortunately they are all pointing to the same value on the parent so the parent also loses that value when those wrapper classes are destroyed.
What we need to do is for each V8 wrapper class that contains other C objects, we need to have private variables that wrap those C objects and when the caller wants to access them we get the value of those private wrappers classes and pass that back to the caller. This way there is only ever one wrapper class for the internal value of the parent and it's also scoped to the parent as well.
The issue with the above example is the 2 console.log lines. Namely:
console.log(commit.author().email()); console.log(commit.author().name());
What's happening here is that each call to commit.author() is creating a wrapper for the author and returning that value. When these go out of scope and garbage collection triggers it will delete the first instance and then try to delete the second. But when it gets to the second instance the value is already deleted so the garbage collector will throw an error. You can actually fix the above example by changing those lines to
var author = commit.author(); console.log(author.email()); console.log(author.name());
Which should run the example with no errors.
I'm closing this issue and starting up a new one for the refactoring of how we store internal objects and pass them back to the caller.
Thanks for the great test case @probablycorey! It helped a lot!
@probablycorey This is fixed in master now.
Awesome, @maxkorp 😄 ‼️
In the lastest version of nodegit I run into malloc errors when walking the commit graph.
How to reproduce
node nodegit-bug.js /Users/corey/code/nodegit
Quirks
If I remove one of the console.log lines the script doesn't fail. If also fails if I console.log parts of the commit like commit.data() or commit.message()