# HG changeset patch # User John W. Eaton # Date 1607997467 18000 # Node ID 76c94c998d7b6ac7f882eb2385a11db331ca9561 # Parent e359e0fcd6e7389c96c8cb66b7a2e39a9f871562 avoid memory leak with function handles (bug #59659) * ov-fcn-handle.h, ov-fcn-handle.cc (octave_fcn_handle::m_rep): Manage memory with std::shared_ptr. Update all uses. diff -r e359e0fcd6e7 -r 76c94c998d7b libinterp/octave-value/ov-fcn-handle.cc --- a/libinterp/octave-value/ov-fcn-handle.cc Sun Dec 13 16:25:03 2020 +0100 +++ b/libinterp/octave-value/ov-fcn-handle.cc Mon Dec 14 20:57:47 2020 -0500 @@ -2568,7 +2568,7 @@ octave_fcn_handle::octave_fcn_handle (const octave_fcn_handle& fh) : octave_base_value (fh) { - m_rep = fh.m_rep->clone (); + m_rep.reset (fh.m_rep->clone ()); } dim_vector @@ -2587,7 +2587,7 @@ bool octave_fcn_handle::load_ascii (std::istream& is) { - octave::base_fcn_handle *new_rep = nullptr; + std::shared_ptr new_rep; // Read enough to detect type then create new rep object and dispatch // to finish loading object. @@ -2632,9 +2632,9 @@ is >> name; if (name == anonymous) - new_rep = new octave::anonymous_fcn_handle (); + new_rep.reset (new octave::anonymous_fcn_handle ()); else - new_rep = new octave::simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::simple_fcn_handle (name, fpath, octaveroot)); } else { @@ -2645,30 +2645,30 @@ std::string name; is >> name; - new_rep = new octave::simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::simple_fcn_handle (name, fpath, octaveroot)); } else if (subtype == "scopedfunction") { std::string name; is >> name; - new_rep = new octave::scoped_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::scoped_fcn_handle (name, fpath, octaveroot)); } else if (subtype == "anonymous") - new_rep = new octave::anonymous_fcn_handle (); + new_rep.reset (new octave::anonymous_fcn_handle ()); else if (subtype == "nested") { std::string name; is >> name; - new_rep = new octave::nested_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::nested_fcn_handle (name, fpath, octaveroot)); } else if (subtype == "classsimple") { std::string name; is >> name; - new_rep = new octave::class_simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::class_simple_fcn_handle (name, fpath, octaveroot)); } } @@ -2676,12 +2676,8 @@ return false; if (! new_rep->load_ascii (is)) - { - delete new_rep; - return false; - } - - delete m_rep; + return false; + m_rep = new_rep; return true; @@ -2716,7 +2712,7 @@ if (! is) return false; - octave::base_fcn_handle *new_rep = nullptr; + std::shared_ptr new_rep; size_t anl = anonymous.length (); @@ -2727,7 +2723,7 @@ // number of local variables appended. We decode that inside the // load_binary function. - new_rep = new octave::anonymous_fcn_handle (name); + new_rep.reset (new octave::anonymous_fcn_handle (name)); } else { @@ -2766,25 +2762,21 @@ // following list. if (subtype == "simple") - new_rep = new octave::simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::simple_fcn_handle (name, fpath, octaveroot)); else if (subtype == "scopedfunction") - new_rep = new octave::scoped_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::scoped_fcn_handle (name, fpath, octaveroot)); else if (subtype == "nested") - new_rep = new octave::nested_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::nested_fcn_handle (name, fpath, octaveroot)); else if (subtype == "classsimple") - new_rep = new octave::class_simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::class_simple_fcn_handle (name, fpath, octaveroot)); } if (! new_rep) return false; if (! new_rep->load_binary (is, swap, fmt)) - { - delete new_rep; - return false; - } - - delete m_rep; + return false; + m_rep = new_rep; return true; @@ -2877,14 +2869,14 @@ std::string name (nm_tmp); - octave::base_fcn_handle *new_rep = nullptr; + std::shared_ptr new_rep; if (name == anonymous) { // Even with extra info stored in the function name, anonymous // functions look the same. - new_rep = new octave::anonymous_fcn_handle (); + new_rep.reset (new octave::anonymous_fcn_handle ()); } else { @@ -2923,27 +2915,21 @@ // following list. if (subtype == "simple") - new_rep = new octave::simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::simple_fcn_handle (name, fpath, octaveroot)); else if (subtype == "scopedfunction") - new_rep = new octave::scoped_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::scoped_fcn_handle (name, fpath, octaveroot)); else if (subtype == "nested") - new_rep = new octave::nested_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::nested_fcn_handle (name, fpath, octaveroot)); else if (subtype == "classsimple") - new_rep = new octave::class_simple_fcn_handle (name, fpath, octaveroot); + new_rep.reset (new octave::class_simple_fcn_handle (name, fpath, octaveroot)); } bool status = false; - if (new_rep) + if (new_rep && new_rep->load_hdf5 (group_hid, space_hid, type_hid)) { - if (new_rep->load_hdf5 (group_hid, space_hid, type_hid)) - { - delete m_rep; - m_rep = new_rep; - status = true; - } - else - delete new_rep; + m_rep = new_rep; + status = true; } // FIXME: manage these with an unwind_action object? diff -r e359e0fcd6e7 -r 76c94c998d7b libinterp/octave-value/ov-fcn-handle.h --- a/libinterp/octave-value/ov-fcn-handle.h Sun Dec 13 16:25:03 2020 +0100 +++ b/libinterp/octave-value/ov-fcn-handle.h Mon Dec 14 20:57:47 2020 -0500 @@ -30,6 +30,7 @@ #include #include +#include #include #include "oct-map.h" @@ -325,11 +326,11 @@ private: - octave::base_fcn_handle *m_rep; + std::shared_ptr m_rep; octave_fcn_handle (octave::base_fcn_handle *rep); - octave::base_fcn_handle * get_rep (void) const { return m_rep; } + octave::base_fcn_handle * get_rep (void) const { return m_rep.get (); } DECLARE_OV_TYPEID_FUNCTIONS_AND_DATA };