Mercurial > octave
changeset 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 | 0574c36a095e |
| children | 34d06c73b48d |
| files | libinterp/corefcn/call-stack.cc libinterp/corefcn/stack-frame.cc libinterp/corefcn/stack-frame.h libinterp/octave-value/cdef-object.cc libinterp/octave-value/cdef-object.h libinterp/octave-value/ov-base.h libinterp/octave-value/ov-cell.cc libinterp/octave-value/ov-cell.h libinterp/octave-value/ov-class.cc libinterp/octave-value/ov-class.h libinterp/octave-value/ov-classdef.h libinterp/octave-value/ov-fcn-handle.cc libinterp/octave-value/ov-fcn-handle.h libinterp/octave-value/ov-struct.cc libinterp/octave-value/ov-struct.h libinterp/octave-value/ov.cc libinterp/octave-value/ov.h |
| diffstat | 17 files changed, 333 insertions(+), 66 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/call-stack.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/corefcn/call-stack.cc Fri Mar 05 16:14:34 2021 -0500 @@ -763,6 +763,9 @@ m_curr_frame = caller->index (); + if (elt->is_closure_context ()) + elt->break_closure_cycles (elt); + m_cs.pop_back (); } }
--- a/libinterp/corefcn/stack-frame.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/corefcn/stack-frame.cc Fri Mar 05 16:14:34 2021 -0500 @@ -477,6 +477,8 @@ void accept (stack_frame_walker& sfw); + void break_closure_cycles (const std::shared_ptr<stack_frame>& frame); + private: // User-defined object associated with this stack frame. Should @@ -2352,6 +2354,15 @@ sfw.visit_user_fcn_stack_frame (*this); } + void user_fcn_stack_frame::break_closure_cycles (const std::shared_ptr<stack_frame>& frame) + { + for (auto& val : m_values) + val.break_closure_cycles (frame); + + if (m_access_link) + m_access_link->break_closure_cycles (frame); + } + symbol_record scope_stack_frame::insert_symbol (const std::string& name) { // There is no access link for scope frames, so there is no other
--- a/libinterp/corefcn/stack-frame.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/corefcn/stack-frame.h Fri Mar 05 16:14:34 2021 -0500 @@ -143,7 +143,8 @@ const std::shared_ptr<stack_frame>& parent_link, const std::shared_ptr<stack_frame>& static_link, const std::shared_ptr<stack_frame>& access_link) - : m_evaluator (tw), m_line (-1), m_column (-1), m_index (index), + : m_evaluator (tw), m_is_closure_context (false), + m_line (-1), m_column (-1), m_index (index), m_parent_link (parent_link), m_static_link (static_link), m_access_link (access_link), m_dispatch_class () { } @@ -550,6 +551,11 @@ virtual void accept (stack_frame_walker& sfw) = 0; + virtual void break_closure_cycles (const std::shared_ptr<stack_frame>&) { } + + void mark_closure_context (void) { m_is_closure_context = true; } + bool is_closure_context (void) const { return m_is_closure_context; } + protected: // Reference to the call stack that contains this frame. Global @@ -557,6 +563,10 @@ // immediate access to them. tree_evaluator& m_evaluator; + // TRUE if this stack frame is saved with a handle to a nested + // function (closure). + bool m_is_closure_context; + // The line and column of the source file where this stack frame // was created. Used to print stack traces. int m_line;
--- a/libinterp/octave-value/cdef-object.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/cdef-object.cc Fri Mar 05 16:14:34 2021 -0500 @@ -499,6 +499,13 @@ } } + void + cdef_object_scalar::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) + { + for (octave_idx_type i = 0; i < map.nfields (); i++) + map.contents(i).break_closure_cycles (frame); + } + octave_value_list cdef_object_scalar::subsref (const std::string& type, const std::list<octave_value_list>& idx,
--- a/libinterp/octave-value/cdef-object.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/cdef-object.h Fri Mar 05 16:14:34 2021 -0500 @@ -119,6 +119,11 @@ err_invalid_object ("get_property"); } + virtual void break_closure_cycles (const std::shared_ptr<octave::stack_frame>&) + { + err_invalid_object ("break_closure_cycles"); + } + virtual octave_value_list subsref (const std::string&, const std::list<octave_value_list>&, int, size_t&, const cdef_class&, bool) @@ -275,6 +280,11 @@ return rep->get_property (idx, pname); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) + { + rep->break_closure_cycles (frame); + } + octave_value_list subsref (const std::string& type, const std::list<octave_value_list>& idx, int nargout, size_t& skip, const cdef_class& context, @@ -455,6 +465,8 @@ dim_vector dims (void) const { return dim_vector (1, 1); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame); + void put (const std::string& pname, const octave_value& val) { map.assign (pname, val);
--- a/libinterp/octave-value/ov-base.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-base.h Fri Mar 05 16:14:34 2021 -0500 @@ -32,6 +32,7 @@ #include <iosfwd> #include <list> +#include <memory> #include <string> #include "Range.h" @@ -44,6 +45,7 @@ namespace octave { + class stack_frame; class type_info; // FIXME: This is not ideal, but it avoids including @@ -247,6 +249,8 @@ virtual octave_base_value * unique_clone (void) { return clone (); } + virtual void break_closure_cycles (const std::shared_ptr<octave::stack_frame>&) { } + virtual type_conv_info numeric_conversion_function (void) const { return type_conv_info (); }
--- a/libinterp/octave-value/ov-cell.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-cell.cc Fri Mar 05 16:14:34 2021 -0500 @@ -50,6 +50,7 @@ #include "utils.h" #include "ov-base-mat.h" #include "ov-base-mat.cc" +#include "ov-fcn-handle.h" #include "ov-re-mat.h" #include "ov-scalar.h" #include "pr-output.h" @@ -143,6 +144,12 @@ DEFINE_OV_TYPEID_FUNCTIONS_AND_DATA (octave_cell, "cell", "cell"); +void octave_cell::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) +{ + for (octave_idx_type i = 0; i < matrix.numel (); i++) + matrix(i).break_closure_cycles (frame); +} + octave_value_list octave_cell::subsref (const std::string& type, const std::list<octave_value_list>& idx,
--- a/libinterp/octave-value/ov-cell.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-cell.h Fri Mar 05 16:14:34 2021 -0500 @@ -69,6 +69,8 @@ octave_base_value * clone (void) const { return new octave_cell (*this); } octave_base_value * empty_clone (void) const { return new octave_cell (); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame); + #if 0 octave_base_value * try_narrowing_conversion (void); #endif
--- a/libinterp/octave-value/ov-class.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-class.cc Fri Mar 05 16:14:34 2021 -0500 @@ -266,6 +266,18 @@ error ("%s cannot be indexed with %c", nm.c_str (), t); } +void +octave_class::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) +{ + for (octave_idx_type j = 0; j < map.nfields (); j++) + { + Cell& c = map.contents (j); + + for (octave_idx_type i = 0; i < c.numel (); i++) + c(i).break_closure_cycles (frame); + } +} + Cell octave_class::dotref (const octave_value_list& idx) {
--- a/libinterp/octave-value/ov-class.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-class.h Fri Mar 05 16:14:34 2021 -0500 @@ -88,6 +88,8 @@ return new octave_class (octave_map (map.keys ()), c_name, parent_list); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame); + Cell dotref (const octave_value_list& idx); Matrix size (void);
--- a/libinterp/octave-value/ov-classdef.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-classdef.h Fri Mar 05 16:14:34 2021 -0500 @@ -89,6 +89,11 @@ bool is_instance_of (const std::string& cls_name) const; + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) + { + object.break_closure_cycles (frame); + } + octave_value_list subsref (const std::string& type, const std::list<octave_value_list>& idx, int nargout);
--- a/libinterp/octave-value/ov-fcn-handle.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-fcn-handle.cc Fri Mar 05 16:14:34 2021 -0500 @@ -312,37 +312,29 @@ std::list<std::string> m_parentage; }; - class nested_fcn_handle : public base_fcn_handle + class base_nested_fcn_handle : public base_fcn_handle { public: // FIXME: octaveroot is temporary information used when loading // handles. Can we avoid using it in the constructor? - nested_fcn_handle (const std::string& name = "", - const std::string& file = "", - const std::string& /*octaveroot*/ = "") + base_nested_fcn_handle (const std::string& name = "", + const std::string& file = "", + const std::string& /*octaveroot*/ = "") : base_fcn_handle (name, file) { } - nested_fcn_handle (const octave_value& fcn, const std::string& name, - const std::shared_ptr<stack_frame>& closure_frames); - - nested_fcn_handle (const nested_fcn_handle&) = default; - - ~nested_fcn_handle (void) = default; - - nested_fcn_handle * clone (void) const - { - return new nested_fcn_handle (*this); - } + base_nested_fcn_handle (const octave_value& fcn, const std::string& name) + : base_fcn_handle (name), m_fcn (fcn) + { } std::string type (void) const { return "nested"; } + using base_fcn_handle::is_nested; + bool is_nested (void) const { return true; } - octave_value_list call (int nargout, const octave_value_list& args); - // FIXME: These must go away. They don't do the right thing for // scoping or overloads. octave_function * function_value (bool = false) @@ -357,7 +349,7 @@ octave_value fcn_val (void) { return m_fcn; } - octave_value workspace (void) const; + virtual octave_value workspace (void) const = 0; // Should be const. octave_scalar_map info (void); @@ -378,16 +370,98 @@ void print_raw (std::ostream&, bool pr_as_read_syntax, int current_print_indent_level) const; - friend bool is_equal_to (const nested_fcn_handle& fh1, - const nested_fcn_handle& fh2); - protected: // The function we are handling. octave_value m_fcn; + }; + + class nested_fcn_handle : public base_nested_fcn_handle + { + public: + + // FIXME: octaveroot is temporary information used when loading + // handles. Can we avoid using it in the constructor? + + nested_fcn_handle (const std::string& name = "", + const std::string& file = "", + const std::string& octaveroot = "") + : base_nested_fcn_handle (name, file, octaveroot) + { } + + nested_fcn_handle (const octave_value& fcn, const std::string& name, + const std::shared_ptr<stack_frame>& stack_context) + : base_nested_fcn_handle (fcn, name), m_stack_context (stack_context) + { + m_stack_context->mark_closure_context (); + } + + nested_fcn_handle (const nested_fcn_handle&) = default; + + ~nested_fcn_handle (void) = default; + + using base_nested_fcn_handle::is_nested; + + bool is_nested (const std::shared_ptr<stack_frame>& frame) const + { + return frame == m_stack_context; + } + + nested_fcn_handle * clone (void) const + { + return new nested_fcn_handle (*this); + } + + octave_value make_weak_nested_handle (void) const; + + octave_value_list call (int nargout, const octave_value_list& args); + + octave_value workspace (void) const; + + friend bool is_equal_to (const nested_fcn_handle& fh1, + const nested_fcn_handle& fh2); + + std::shared_ptr<stack_frame> stack_context (void) const + { + return m_stack_context; + } + + protected: // Pointer to closure stack frames. - std::shared_ptr<stack_frame> m_closure_frames; + std::shared_ptr<stack_frame> m_stack_context; + }; + + class weak_nested_fcn_handle : public base_nested_fcn_handle + { + public: + + weak_nested_fcn_handle (const nested_fcn_handle& nfh) + : base_nested_fcn_handle (nfh), m_stack_context (nfh.stack_context ()) + { } + + weak_nested_fcn_handle (const weak_nested_fcn_handle&) = default; + + ~weak_nested_fcn_handle (void) = default; + + weak_nested_fcn_handle * clone (void) const + { + return new weak_nested_fcn_handle (*this); + } + + bool is_weak_nested (void) const { return true; } + + octave_value_list call (int nargout, const octave_value_list& args); + + octave_value workspace (void) const; + + friend bool is_equal_to (const weak_nested_fcn_handle& fh1, + const weak_nested_fcn_handle& fh2); + + protected: + + // Pointer to closure stack frames. + std::weak_ptr<stack_frame> m_stack_context; }; class class_simple_fcn_handle : public base_fcn_handle @@ -597,6 +671,13 @@ name.c_str ()); } + octave_value base_fcn_handle::make_weak_nested_handle (void) const + { + std::string type_str = type (); + error ("invalid conversion from %s handle to weak nestead handle", + type_str.c_str ()); + } + octave_value_list base_fcn_handle::subsref (const std::string& type, const std::list<octave_value_list>& idx, @@ -1501,32 +1582,7 @@ } } - nested_fcn_handle::nested_fcn_handle (const octave_value& fcn, - const std::string& name, - const std::shared_ptr<stack_frame>& closure_frames) - : base_fcn_handle (name), m_fcn (fcn), m_closure_frames (closure_frames) - { } - - octave_value_list - nested_fcn_handle::call (int nargout, const octave_value_list& args) - { - tree_evaluator& tw = __get_evaluator__ ("nested_fcn_handle::call"); - - octave_user_function *oct_usr_fcn = m_fcn.user_function_value (); - - tw.push_stack_frame (oct_usr_fcn, m_closure_frames); - - unwind_action act ([&tw] () { tw.pop_stack_frame (); }); - - return oct_usr_fcn->execute (tw, nargout, args); - } - - octave_value nested_fcn_handle::workspace (void) const - { - return m_closure_frames->workspace (); - } - - octave_scalar_map nested_fcn_handle::info (void) + octave_scalar_map base_nested_fcn_handle::info (void) { octave_scalar_map m; @@ -1547,7 +1603,7 @@ // Is it an error if that fails? Or should this job always be // deferred until the handle is used? - bool nested_fcn_handle::save_ascii (std::ostream& os) + bool base_nested_fcn_handle::save_ascii (std::ostream& os) { unimplemented ("save", "text"); @@ -1556,7 +1612,7 @@ return true; } - bool nested_fcn_handle::load_ascii (std::istream& is) + bool base_nested_fcn_handle::load_ascii (std::istream& is) { unimplemented ("load", "text"); @@ -1565,7 +1621,8 @@ return true; } - bool nested_fcn_handle::save_binary (std::ostream& os, bool save_as_floats) + bool base_nested_fcn_handle::save_binary (std::ostream& os, + bool save_as_floats) { unimplemented ("save", "binary"); @@ -1575,8 +1632,8 @@ return true; } - bool nested_fcn_handle::load_binary (std::istream& is, bool swap, - mach_info::float_format fmt) + bool base_nested_fcn_handle::load_binary (std::istream& is, bool swap, + mach_info::float_format fmt) { unimplemented ("load", "binary"); @@ -1587,8 +1644,8 @@ return true; } - bool nested_fcn_handle::save_hdf5 (octave_hdf5_id loc_id, const char *name, - bool) + bool base_nested_fcn_handle::save_hdf5 (octave_hdf5_id loc_id, + const char *name, bool) { #if defined (HAVE_HDF5) @@ -1611,9 +1668,9 @@ #endif } - bool nested_fcn_handle::load_hdf5 (octave_hdf5_id& group_hid, - octave_hdf5_id& space_hid, - octave_hdf5_id& type_hid) + bool base_nested_fcn_handle::load_hdf5 (octave_hdf5_id& group_hid, + octave_hdf5_id& space_hid, + octave_hdf5_id& type_hid) { #if defined (HAVE_HDF5) @@ -1636,14 +1693,39 @@ #endif } - void nested_fcn_handle::print_raw (std::ostream& os, - bool pr_as_read_syntax, - int current_print_indent_level) const + void base_nested_fcn_handle::print_raw (std::ostream& os, + bool pr_as_read_syntax, + int current_print_indent_level) const { octave_print_internal (os, '@' + m_name, pr_as_read_syntax, current_print_indent_level); } + octave_value nested_fcn_handle::make_weak_nested_handle (void) const + { + return octave_value (new octave_fcn_handle + (new weak_nested_fcn_handle (*this))); + } + + octave_value_list + nested_fcn_handle::call (int nargout, const octave_value_list& args) + { + tree_evaluator& tw = __get_evaluator__ ("nested_fcn_handle::call"); + + octave_user_function *oct_usr_fcn = m_fcn.user_function_value (); + + tw.push_stack_frame (oct_usr_fcn, m_stack_context); + + unwind_action act ([&tw] () { tw.pop_stack_frame (); }); + + return oct_usr_fcn->execute (tw, nargout, args); + } + + octave_value nested_fcn_handle::workspace (void) const + { + return m_stack_context->workspace (); + } + bool is_equal_to (const nested_fcn_handle& fh1, const nested_fcn_handle& fh2) { if (fh1.m_name == fh2.m_name @@ -1653,6 +1735,39 @@ return false; } + octave_value_list + weak_nested_fcn_handle::call (int nargout, const octave_value_list& args) + { + tree_evaluator& tw = __get_evaluator__ ("weak_nested_fcn_handle::call"); + + octave_user_function *oct_usr_fcn = m_fcn.user_function_value (); + + std::shared_ptr<stack_frame> frames = m_stack_context.lock (); + + tw.push_stack_frame (oct_usr_fcn, frames); + + unwind_action act ([&tw] () { tw.pop_stack_frame (); }); + + return oct_usr_fcn->execute (tw, nargout, args); + } + + octave_value weak_nested_fcn_handle::workspace (void) const + { + std::shared_ptr<stack_frame> frames = m_stack_context.lock (); + + return frames ? frames->workspace () : octave_value (); + } + + bool is_equal_to (const weak_nested_fcn_handle& fh1, + const weak_nested_fcn_handle& fh2) + { + if (fh1.m_name == fh2.m_name + && fh1.m_fcn.is_defined () && fh2.m_fcn.is_defined ()) + return fh1.m_fcn.is_copy_of (fh2.m_fcn); + else + return false; + } + class_simple_fcn_handle::class_simple_fcn_handle (const std::string& class_nm, const std::string& meth_nm) : base_fcn_handle (meth_nm), m_obj (), m_fcn (), @@ -2554,9 +2669,9 @@ octave_fcn_handle::octave_fcn_handle (const octave_value& fcn, const std::string& name, - const std::shared_ptr<octave::stack_frame>& closure_frames) + const std::shared_ptr<octave::stack_frame>& stack_context) : octave_base_value (), - m_rep (new octave::nested_fcn_handle (fcn, name, closure_frames)) + m_rep (new octave::nested_fcn_handle (fcn, name, stack_context)) { } octave_fcn_handle::octave_fcn_handle (const octave_value& fcn,
--- a/libinterp/octave-value/ov-fcn-handle.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-fcn-handle.h Fri Mar 05 16:14:34 2021 -0500 @@ -72,10 +72,19 @@ virtual bool is_nested (void) const { return false; } + virtual bool is_nested (const std::shared_ptr<stack_frame>&) const + { + return false; + } + + virtual bool is_weak_nested (void) const { return false; } + virtual bool is_class_simple (void) const { return false; } virtual bool is_anonymous (void) const { return false; } + virtual octave_value make_weak_nested_handle (void) const; + std::string fcn_name (void) const { return m_name; } std::string file (void) const { return m_file; } @@ -205,6 +214,8 @@ octave_fcn_handle (const octave_value& fcn, const octave::stack_frame::local_vars_map& local_vars); + octave_fcn_handle (octave::base_fcn_handle *rep); + octave_fcn_handle (const octave_fcn_handle& fh); ~octave_fcn_handle (void) = default; @@ -254,10 +265,22 @@ bool is_nested (void) const { return m_rep->is_nested (); } + bool is_nested (const std::shared_ptr<octave::stack_frame>& frame) const + { + return m_rep->is_nested (frame); + } + + bool is_weak_nested (void) const { return m_rep->is_weak_nested (); } + bool is_class_simple (void) const { return m_rep->is_class_simple (); } bool is_anonymous (void) const { return m_rep->is_anonymous (); } + octave_value make_weak_nested_handle (void) const + { + return m_rep->make_weak_nested_handle (); + } + dim_vector dims (void) const; // FIXME: These must go away. They don't do the right thing for @@ -328,8 +351,6 @@ std::shared_ptr<octave::base_fcn_handle> m_rep; - octave_fcn_handle (octave::base_fcn_handle *rep); - octave::base_fcn_handle * get_rep (void) const { return m_rep.get (); } DECLARE_OV_TYPEID_FUNCTIONS_AND_DATA
--- a/libinterp/octave-value/ov-struct.cc Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-struct.cc Fri Mar 05 16:14:34 2021 -0500 @@ -63,6 +63,18 @@ // specified by struct_levels_to_print. static bool Vprint_struct_array_contents = false; +void +octave_struct::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) +{ + for (octave_idx_type j = 0; j < map.nfields (); j++) + { + Cell& c = map.contents (j); + + for (octave_idx_type i = 0; i < c.numel (); i++) + c(i).break_closure_cycles (frame); + } +} + octave_base_value * octave_struct::try_narrowing_conversion (void) { @@ -1085,6 +1097,13 @@ DEFINE_OV_TYPEID_FUNCTIONS_AND_DATA(octave_scalar_struct, "scalar struct", "struct"); +void +octave_scalar_struct::break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame) +{ + for (octave_idx_type i = 0; i < map.nfields (); i++) + map.contents(i).break_closure_cycles (frame); +} + octave_value octave_scalar_struct::dotref (const octave_value_list& idx, bool auto_add) {
--- a/libinterp/octave-value/ov-struct.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov-struct.h Fri Mar 05 16:14:34 2021 -0500 @@ -64,6 +64,8 @@ octave_base_value * clone (void) const { return new octave_struct (*this); } octave_base_value * empty_clone (void) const { return new octave_struct (); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame); + octave_base_value * try_narrowing_conversion (void); Cell dotref (const octave_value_list& idx, bool auto_add = false); @@ -192,6 +194,8 @@ octave_base_value * empty_clone (void) const { return new octave_scalar_struct (); } + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>& frame); + octave_value dotref (const octave_value_list& idx, bool auto_add = false); octave_value subsref (const std::string& type,
--- 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 ();
--- a/libinterp/octave-value/ov.h Tue Mar 02 12:32:43 2021 -0500 +++ b/libinterp/octave-value/ov.h Fri Mar 05 16:14:34 2021 -0500 @@ -33,6 +33,7 @@ #include <iosfwd> #include <string> #include <list> +#include <memory> #include <map> #include "Range.h" @@ -46,6 +47,7 @@ namespace octave { + class stack_frame; class type_info; } @@ -362,6 +364,12 @@ } } + // Convert any nested function handles in this object to use weak + // references to their enclosing stack frame context. Used to break + // shared_ptr reference cycles for handles to nested functions + // (closures). + void break_closure_cycles (const std::shared_ptr<octave::stack_frame>&); + // Simple assignment. octave_value& operator = (const octave_value& a)