Closed maxirmx closed 6 months ago
jasonroelofs
commented
6 months ago I wonder if there's something going on between the interplay of Animal and AnimalProxy. Why do you have the proxy object? Does this still crash if you return lists of Animal itself?
maxirmx
commented
6 months ago Hi, @jasonroelofs Thank you for looking at this issue.
Let me explain a background. I am trying to stabilize expressir gem. It has native extension is C++ generated by antlr4-native gem that uses Rice gem to wrap code generated by antlr4 parser generator. These are ~20K lines of generated C++ code that keeps crashing unpredictably.
I have AnimalProxy in the sample because it resembles the structure of production code. There Animal is a virtual base class with its own hierarchy on top and AnimalProxy is a decorator that adds some functionality.
If I remove AnimalProxy and return Animal itself it still crashes.
maxirmx
commented
6 months ago If I can make a suggestion regarding the crash, here it follows:
template<typename From_Ruby_T, typename Function_T, bool IsMethod>
void NativeFunction<From_Ruby_T, Function_T, IsMethod>::checkKeepAlive(VALUE self, VALUE returnValue, std::vector<VALUE>& rubyValues)
{
// Check function arguments
Wrapper* selfWrapper = getWrapper(self);
for (const Arg& arg : (*this->methodInfo_))
{
if (arg.isKeepAlive())
{
selfWrapper->addKeepAlive(rubyValues[arg.position]);
}
}
// Check return value
if (this->methodInfo_->returnInfo.isKeepAlive())
{
Wrapper* returnWrapper = getWrapper(returnValue);
returnWrapper->addKeepAlive(self);
}
}The function above calls getWrapper and getWrapper is just
inline Wrapper* getWrapper(VALUE value)
{
return static_cast<Wrapper*>(RTYPEDDATA_DATA(value));
}However, in my sample returnValue is an Array (built-in type), not wrapped type so the cast above will have unpredictable results.
jasonroelofs
commented
6 months ago I've played around with your example here and looked more into what you're doing and the intended use of Return, and I don't think you need to use Return for either of these methods, particularly get_pets because every time it's called you are creating a new Array anyway. Leaving out Return, or making it more explicit with Return.takeOwnership() seem to work fine and do not cause memory leaks that I can find.
Granted, it probably shouldn't crash at all, but I would need to look further into the nitty gritty of what's being stored here to figure out what's getting messed up (I'm guessing a value isn't getting marked for GC and is getting prematurely freed).
Possibly something else is going on with the expressir gem? Do you have an example crash you're trying to fix?
maxirmx
commented
6 months ago Thank you, @jasonroelofs
Every time I call get_pets it returns an Array that refers objects "embedded" into Zoo object. That's why I would like to mark Zoo object from the object retuned by get_pets. In other words I am not concerned that the Array would be prematurely garbade collected. I am concerned that Zoo is destroyed but there will be references to its internal in Ruby.
It may look like overkill for this sample but I am trying to create simple case from a very large extension (~20 000 lines)
It crashes because getWrapper function assumes that the return value is wrapped type and behaves incorrectly if return value is native type. Here is a PR that adresses this issue https://github.com/jasonroelofs/rice/pull/194
I cannot say that it is a real fix but at least it throws an exception and does not crash.
You are right, there is something else happeninng in expressir gem. I was trying to keep Array alive but after looking at comments here https://github.com/jasonroelofs/rice/commit/b4871797e9821339dda0c169262f824c413406a3 I think that there may be "entries in the array are getting GC'd"
jasonroelofs
commented
6 months ago Fixed with #194
Hello, Rice developers.
Thank you very much for making a great library. It has always worked like charm after reading documentation but now I am facing an issue that I cannot explain. This example is a minimized version of data structure generated by https://github.com/camertron/antlr4-native-rb[antlr4-native] gem
This code implements method(s) that return arrays of items created by and inside C++ extension like getPets and getPets2 functions. In my example getPets returns Rice::Array and getPets2 returns wrapped std::vector. Both functions work until I apply Return.keepAlive(). With keepAlive the function that returns Rice::Array crashes
Any help will be much appreciated
Extension:
Spec:
Crash dump
Output withous GC.stress = true