Mercurial > octave
changeset 20771:ed708a7ce0a2
simplify memory management for mex files (bug #46559)
Now that errors throw exceptions we can avoid using setjmp/longjmp and
unwind_protect to manage memory allocations that must be deleted
automatically in MEX functions.
* mex.cc (mex::~mex): Delete memlist and arraylist objects here.
(mex::cleanup, mex:jump): Delete.
(mex::abort): Delete. Remove all uses.
(call_mex): Call octave_quit at beginning of function.
Don't add mex::cleanup to unwind_protect stack.
Don't use setjmp. Don't run unwind_protect stack explicitly.
(mexCallMATLAB): Propagate original exception unless trapping feval
error.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Fri, 27 Nov 2015 16:00:30 -0500 |
parents | 826a4771718b |
children | 4d78e076a592 |
files | libinterp/corefcn/mex.cc |
diffstat | 1 files changed, 44 insertions(+), 75 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/mex.cc Fri Nov 27 14:10:35 2015 -0500 +++ b/libinterp/corefcn/mex.cc Fri Nov 27 16:00:30 2015 -0500 @@ -2016,7 +2016,23 @@ ~mex (void) { - if (! memlist.empty ()) + // We can't use mex::free here because it modifies memlist. + while (! memlist.empty ()) + { + std::set<void *>::iterator p = memlist.begin (); + xfree (*p); + memlist.erase (p); + } + + // We can't use mex::free_value here because it modifies arraylist. + while (! arraylist.empty ()) + { + std::set<mxArray *>::iterator p = arraylist.begin (); + delete *p; + arraylist.erase (p); + } + + if (! (memlist.empty () && arraylist.empty ())) error ("mex: %s: cleanup failed", function_name ()); mxFree (fname); @@ -2040,26 +2056,6 @@ return fname; } - // Free all unmarked pointers obtained from malloc and calloc. - static void cleanup (void *ptr) - { - mex *context = static_cast<mex *> (ptr); - - // We can't use mex::free here because it modifies memlist. - for (std::set<void *>::iterator p = context->memlist.begin (); - p != context->memlist.end (); p++) - xfree (*p); - - context->memlist.clear (); - - // We can't use mex::free_value here because it modifies arraylist. - for (std::set<mxArray *>::iterator p = context->arraylist.begin (); - p != context->arraylist.end (); p++) - delete *p; - - context->arraylist.clear (); - } - // Allocate memory. void *malloc_unmarked (size_t n) { @@ -2071,8 +2067,6 @@ error ("%s: failed to allocate %d bytes of memory", function_name (), n); - - abort (); } global_mark (ptr); @@ -2272,12 +2266,6 @@ // 1 if error should be returned to MEX file, 0 if abort. int trap_feval_error; - // longjmp return point if mexErrMsgTxt or error. - jmp_buf jump; - - // Trigger a long jump back to the mex calling function. - void abort (void) { longjmp (jump, 1); } - private: // Pointer to the mex function that corresponds to this mex context. @@ -3008,6 +2996,8 @@ call_mex (bool have_fmex, void *f, const octave_value_list& args, int nargout_arg, octave_mex_function *curr_mex_fcn) { + octave_quit (); + // Use at least 1 for nargout since even for zero specified args, // still want to be able to return an ans. @@ -3030,30 +3020,25 @@ mex context (curr_mex_fcn); - frame.add_fcn (mex::cleanup, static_cast<void *> (&context)); - for (int i = 0; i < nargin; i++) argin[i] = context.make_value (args(i)); - if (setjmp (context.jump) == 0) + mex_context = &context; + + if (have_fmex) { - mex_context = &context; - - if (have_fmex) - { - fmex_fptr fcn = FCN_PTR_CAST (fmex_fptr, f); - - int tmp_nargout = nargout; - int tmp_nargin = nargin; - - fcn (tmp_nargout, argout, tmp_nargin, argin); - } - else - { - cmex_fptr fcn = FCN_PTR_CAST (cmex_fptr, f); - - fcn (nargout, argout, nargin, argin); - } + fmex_fptr fcn = FCN_PTR_CAST (fmex_fptr, f); + + int tmp_nargout = nargout; + int tmp_nargin = nargin; + + fcn (tmp_nargout, argout, tmp_nargin, argin); + } + else + { + cmex_fptr fcn = FCN_PTR_CAST (cmex_fptr, f); + + fcn (nargout, argout, nargin, argin); } // Convert returned array entries back into octave values. @@ -3071,9 +3056,6 @@ for (int i = 0; i < nargout; i++) retval(i) = mxArray::as_octave_value (argout[i]); - // Clean up mex resources. - frame.run (); - return retval; } @@ -3094,8 +3076,7 @@ // FIXME: do we need unwind protect to clean up args? Off hand, I // would say that this problem is endemic to Octave and we will // continue to have memory leaks after Ctrl-C until proper exception - // handling is implemented. longjmp() only clears the stack, so any - // class which allocates data on the heap is going to leak. + // handling is implemented. args.resize (nargin); @@ -3112,23 +3093,15 @@ } catch (const octave_execution_exception&) { - recover_from_exception (); - - execution_error = true; - } - - if (execution_error && mex_context->trap_feval_error == 0) - { - // FIXME: is this the correct way to clean up? abort() is - // going to trigger a long jump, so the normal class destructors - // will not be called. Hopefully this will reduce things to a - // tiny leak. Maybe create a new octave memory tracer type - // which prints a friendly message every time it is - // created/copied/deleted to check this. - - args.resize (0); - retval.resize (0); - mex_context->abort (); + if (mex_context->trap_feval_error) + recover_from_exception (); + else + { + args.resize (0); + retval.resize (0); + + throw; + } } int num_to_copy = retval.length (); @@ -3197,8 +3170,6 @@ // Octave's error routine requires a non-null input so use a SPACE. error (" "); } - - mex_context->abort (); } void @@ -3221,8 +3192,6 @@ // Octave's error routine requires a non-null input so use a SPACE. error (" "); } - - mex_context->abort (); } void