Memory leak when exceptions are raised

[ankane]

Hi, it looks like there's a memory leak when functions/methods raise an exception. I'm seeing this with Faiss Ruby with rb_check_frozen (CI logs) as well as the minimal function below:

Rice::define_global_function("hello", []() {
  throw std::runtime_error("error");
});

[cfis]

Are you raising enough exceptions that it would matter?

The code is here - https://github.com/ruby-rice/rice/blob/master/rice/detail/cpp_protect.hpp#L29. It catches an exception, finds the default handler and calls it. The default handler then reraises the exception.

I don't see any obvious problem. Maybe comment out the handler and see if that makes a difference?

[ankane]

It looks like raising exceptions with Ruby C functions (like what's happening in cpp_protect) is skipping destructors in the function dispatching logic.

[cfis]

By function dispatching logic, do you mean NativeFunction, NativeMethod, etc?

The cpp_protect block starts immediately after Ruby calls into C++ at https://github.com/ruby-rice/rice/blob/master/rice/detail/Native.ipp#L50. Any C++ objects below that should have their destructors invoked on an exception when the call stack is unwound.

Once the exception gets caught by the catch block, then Rice hands off to Ruby which will do a longjmp. That won't call destructors, but there shouldn't be any to call at that point.

What C++ objects are you seeing whose destructors are not being called?

[ankane]

Yeah, but now I think it may only be occurring with unprotected Ruby calls rather than C++ exceptions, which is solved by using Rice::detail::protect.

#include <rice/rice.hpp>

extern "C"
void Init_ext() {
  Rice::define_module("Hello")
    .define_singleton_function("world", [](Rice::Object o) {
      rb_check_frozen(o.value()); // should use Rice::detail::protect
    });
}

require_relative "ext"

Hello.world("!!".freeze) rescue nil

Valgrind output (leaked objects in getRubyValues and readRubyArgs)

16 bytes in 1 blocks are definitely lost in loss record 2 of 38
  operator new(unsigned long) (at /usr/libexec/valgrind/vgpreload_memcheck-arm64-linux.so)
 *std::__new_allocator<std::optional<unsigned long> >::allocate(unsigned long, void const*) (at /home/ubuntu/code/ext.so)
 *std::_Vector_base<std::optional<unsigned long>, std::allocator<std::optional<unsigned long> > >::_M_allocate(unsigned long) (at /home/ubuntu/code/ext.so)
 *std::_Vector_base<std::optional<unsigned long>, std::allocator<std::optional<unsigned long> > >::_M_create_storage(unsigned long) (at /home/ubuntu/code/ext.so)
 *std::_Vector_base<std::optional<unsigned long>, std::allocator<std::optional<unsigned long> > >::_Vector_base(unsigned long, std::allocator<std::optional<unsigned long> > const&) (at /home/ubuntu/code/ext.so)
 *std::vector<std::optional<unsigned long>, std::allocator<std::optional<unsigned long> > >::vector(unsigned long, std::allocator<std::optional<unsigned long> > const&) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::getRubyValues(std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, unsigned long, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >, bool) (at /home/ubuntu/code/ext.so)
 *Rice::detail::NativeFunction<Init_ext::{lambda(Rice::Object)#1}, false>::operator()(std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, unsigned long, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >&, unsigned long) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}::operator()() const (at /home/ubuntu/code/ext.so)
 *auto Rice::detail::cpp_protect<Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}>(Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}&&) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::resolve(int, unsigned long*, unsigned long) (at /home/ubuntu/code/ext.so)
  vm_call_cfunc_with_frame_ (vm_insnhelper.c:3902)
  vm_sendish.constprop.0 (vm_insnhelper.c:6124)
  vm_exec_core (insns.def:904)
  rb_vm_exec (vm.c:2798)
  rb_ec_exec_node (eval.c:283)
  ruby_run_node (eval.c:321)
  rb_main (main.c:42)
  main (main.c:62)

72 bytes in 1 blocks are definitely lost in loss record 17 of 38
  operator new(unsigned long) (at /usr/libexec/valgrind/vgpreload_memcheck-arm64-linux.so)
 *std::__new_allocator<std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::allocate(unsigned long, void const*) (at /home/ubuntu/code/ext.so)
 *std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::_M_get_node() (at /home/ubuntu/code/ext.so)
 *std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >* std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::_M_create_node<std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, std::tuple<> >(std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::tuple<>&&) (at /home/ubuntu/code/ext.so)
 *std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::_Auto_node::_Auto_node<std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, std::tuple<> >(std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >&, std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::tuple<>&&) (at /home/ubuntu/code/ext.so)
 *std::_Rb_tree_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, std::tuple<> >(std::_Rb_tree_const_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> >, std::piecewise_construct_t const&, std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::tuple<>&&) (at /home/ubuntu/code/ext.so)
 *std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, unsigned long, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, unsigned long> > >::operator[](std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::readRubyArgs[abi:cxx11](unsigned long, unsigned long const*) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}::operator()() const (at /home/ubuntu/code/ext.so)
 *auto Rice::detail::cpp_protect<Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}>(Rice::detail::Native::resolve(int, unsigned long*, unsigned long)::{lambda()#1}&&) (at /home/ubuntu/code/ext.so)
 *Rice::detail::Native::resolve(int, unsigned long*, unsigned long) (at /home/ubuntu/code/ext.so)
  vm_call_cfunc_with_frame_ (vm_insnhelper.c:3902)
  vm_sendish.constprop.0 (vm_insnhelper.c:6124)
  vm_exec_core (insns.def:904)
  rb_vm_exec (vm.c:2798)
  rb_ec_exec_node (eval.c:283)
  ruby_run_node (eval.c:321)
  rb_main (main.c:42)
  main (main.c:62)

I'm still seeing unbounded memory growth with the initial example and the following script:

require_relative "ext"

loop do
  GC.start if rand < 0.01

  hello rescue nil
end

but no Valgrind issues, so not sure about the cause yet.

[cfis]

Thanks for digging in and providing an example script! I'll take a look.

[cfis]

Ok, I added this to the test suite. I'm not seeing memory growth:

I suppose its possible this acts differently since the test suite embeds Ruby, but I would be surprised if that made any difference (since this setup works fine for testing the Ruby GC).

I suppose its also possible Ruby 4.0 acts differently, or a build on Windows acts differently. But again, that seems unlikely. I can try this test on Fedora to see if it is any different.