diff libinterp/octave-value/ov.cc @ 29476:c74ff452e2bb stable

avoid memory leaks when returning handles to nested functions When a handle to a nested function is created with a std::shared_ptr object holding a reference to the stack frames associated with the parent function, we create a circular reference because the referenced stack frame contains the function handle object. To break the link, we'll scan the stack frame associated with the nested function handle and convert the shared_ptr links to be weak_ptr links instead, breaking the circular reference allowing the resources held by the closure frames to be released when all external references are cleared. Another possible solution to this problem is to create weak_ptr links initially and then convert them to shared_ptr links when the closure "escapes" the scope of the function. While that might be slightly more efficient (we would only have to look at assignments to global variables and the values that are actually returned from the function) the current design of global assignments does not make that easy because it is possible to ask for a reference to a global variable, so either all places where we grab references and then later perform assignments must be checked for assignments of handles to nested functions, or we must eliminate the interface that returns references to global variables and only allow assignments so that we would only have to perform the check in the assignment function(s). Perhaps this can be done in a future version, but for now, it works well enough to break the closure cycles in local variables when a function returns. * call-stack.cc (call_stack::pop): If popped stack frame is a closure context, then break closure cycles. * stack-frame.h, stack-frame.cc (stack_frame::m_is_closure_context): New member variable. (stack_frame::break_closure_cycles, user_fcn_stack_frame::break_closure_cycles): New functions. (stack_frame::mark_closure_context, stack_frame::is_closure_context): New functions. * ov-fcn-handle.h, ov-fcn-handle.cc (base_nested_fcn_handle): New base class for handles to nested functions. (nested_fcn_handle, weak_nested_fcn_handle): New classes to represent handles to nested functions. Currently, all handles to nested functions begin as nested_fcn_handle objects but are converted to weak_nested_fcn_handles when the functions where they are created return. (base_fcn_handle::is_nested (const std::shared_ptr<stack_frame>&) const, base_fcn_handle::make_weak_nested_handle, New virtual functions. (octave_fcn_handle::is_nested, octave_fcn_handle::is_weak_nested, octave_fcn_handle::make_weak_nested_handle): New functions. * ov-struct.h, ov-struct.cc (octave_scalar_map::break_closure_cycles, octave_map::break_closure_cycles): New functions. * ov.h, ov.cc (octave_value::break_closure_cycles): New function. * ov-base.h (octave_base_value::break_closure_cycles): New virtual function. * cdef-object.h (cdef_object::break_closure_cycles): New function. (cdef_object_rep::break_closure_cycles): New virtual function. * cdef-object.h, cdef-object.cc (cdef_object_scalar::break_closure_cycles): New function. * ov-cell.h, ov-cell.cc (octave_cell::break_closure_cycles): New function. * ov-class.h, ov-class.cc (octave_class::break_closure_cycles): New function. * ov-classdef.h (octave_classdef::break_closure_cycles): New function.
author John W. Eaton <jwe@octave.org>
date Fri, 05 Mar 2021 16:14:34 -0500
parents 0a5b15007766
children 34d06c73b48d
line wrap: on
line diff
--- a/libinterp/octave-value/ov.cc	Tue Mar 02 12:32:43 2021 -0500
+++ b/libinterp/octave-value/ov.cc	Fri Mar 05 16:14:34 2021 -0500
@@ -1129,6 +1129,31 @@
 }
 
 void
+octave_value::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame)
+{
+  if (is_function_handle ())
+    {
+      octave_fcn_handle *fhdl = rep->fcn_handle_value ();
+
+      if (fhdl->is_nested (frame) && ! fhdl->is_weak_nested ())
+        *this = fhdl->make_weak_nested_handle ();
+    }
+  else
+    {
+      // FIXME: Is there a efficient way to avoid calling make_unique
+      // if REP doesn't contain any nested function handles?
+      //
+      // Probably we should be asking REP to make a modified copy IFF it
+      // is needed, then replace our REP with that if a copy is made,
+      // otherwise we leave it alone.
+
+      make_unique ();
+
+      rep->break_closure_cycles (frame);
+    }
+}
+
+void
 octave_value::maybe_mutate (void)
 {
   octave_base_value *tmp = rep->try_narrowing_conversion ();