search

ruby-rice / rice

Ruby Interface for C++ Extensions
http://ruby-rice.github.io/
Other
378 stars 63 forks source link

Arg::keepAlive can be ignored on exception #185

Open uvlad7 opened 1 year ago

uvlad7 commented 1 year ago

checkKeepAlive is called after invokeNativeFunction/invokeNativeMethod. So, imagine a native function that caches its argument and then throws an exception for some reason. In that case NativeFunction<>::call will handle the exception, but NativeFunction<>::operator() will fail to store the argument into keepAlive_. It's not an issue for me, actually, but I think it's worth highlighting in the documentation or fixing it.

cfis commented 9 months ago

If an exception is raised in invokeNativeFunction/invokeNativeMethod then there would be no result returned from either. So seems ok for return values marked as keepAlive since there is nothing to mark.

As for args marked as keep alive, I am not sure what the right thing would be. The code could process those before invoking the function...but what if the function never got far enough to actually take ownership of the passed in arg? I guess better to call mark on it and be safe then not do it and possibly GC it on the Ruby side?

uvlad7 commented 9 months ago

I'm also not sure what to do with it, probably just highlight the pitfall in the docs is OK. Usually this has no effect.

cfis commented 9 months ago

Another thing that crossed my mind is there no API removeKeepAlive. For example, if you remove an item from a collection. Have you ran into that issue?

uvlad7 commented 9 months ago

No, I never needed that. Actually, I used Arg::keepAlive only in constructor and have never ran into keepAlive on exception issue, too. I just noticed it can happen.