bugGNU Octave - Bugs: bug #62459, Severe memory leak when functions...

 
 

bug #62459: Severe memory leak when functions are nested

Submitter:  PIERRE LABRECHE <pierre5018>
Submitted:  Sat 14 May 2022 04:34:24 PM UTC
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  PIERRE LABRECHE Open/Closed:  * Closed
Release:  * 7.1.0 Operating System:  * Any
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Tue 24 May 2022 06:40:18 PM UTC, comment #21: 

I verified the patch and made it a complete changeset here: http://hg.savannah.gnu.org/hgweb/octave/rev/490e8a6107d0.

This will be a part of the 7.2 release.

Marking as Fixed and closing report.

Rik <rik5>
Project Administrator
Tue 24 May 2022 05:46:09 PM UTC, comment #20: 

Oops.  The member variable m_stack_context is a (shared) pointer that could be nullptr so we should check it before using.  Does the attached change work for you?  It seems to pass all the tests on stable for me.

(file #53252)

John W. Eaton <jwe>
Project Administrator
Tue 24 May 2022 04:48:41 PM UTC, comment #19: 

Follow-up to comment #18, any anonymous function seems to cause a segfault.  It's not a special case of an anonymous function with no inputs.

Rik <rik5>
Project Administrator
Tue 24 May 2022 04:15:37 PM UTC, comment #18: 

With the clue from gdb, here is a simple test case

octave:1> f = @() 1;
fatal: caught signal Segmentation fault -- stopping myself...
Segmentation fault (core dumped)

Rik <rik5>
Project Administrator
Tue 24 May 2022 04:14:53 PM UTC, comment #17: 

Backtrace generated from development sources:

(gdb) bt
#0  octave::anonymous_fcn_handle::anonymous_fcn_handle (this=0x7f5c34565050, fcn=...,
    local_vars=std::map with 0 elements, stack_context=std::shared_ptr<octave::stack_frame> (empty) = {...})
    at libinterp/octave-value/ov-fcn-handle.cc:2701
#1  0x00007f5c55026d5c in octave_fcn_handle::octave_fcn_handle (this=this@entry=0x7f5c34669040, fcn=...,
    local_vars=std::map with 0 elements, stack_context=std::shared_ptr<octave::stack_frame> (empty) = {...})
    at libinterp/octave-value/ov-fcn-handle.cc:2879
#2  0x00007f5c551b6852 in octave::tree_evaluator::evaluate_anon_fcn_handle (this=0x7f5c340058b8, afh=...)
    at libinterp/parse-tree/pt-eval.cc:3421
#3  0x00007f5c551bbbeb in octave::tree_anon_fcn_handle::evaluate (this=<optimized out>, tw=...)
    at libinterp/parse-tree/pt-fcn-handle.cc:104
#4  0x00007f5c55193481 in octave::tree_simple_assignment::evaluate (this=0x7f5c34679130, tw=...)
    at libinterp/parse-tree/pt-assign.cc:101
#5  0x00007f5c551b48ad in octave::tree_evaluator::visit_statement (this=0x7f5c340058b8, stmt=...)
    at libinterp/parse-tree/pt-eval.cc:3940
#6  0x00007f5c551a069c in octave::tree_statement::accept (tw=..., this=0x7f5c346421f0)
    at libinterp/parse-tree/pt-stmt.h:124
#7  octave::tree_evaluator::visit_statement_list (this=0x7f5c340058b8, lst=...) at libinterp/parse-tree/pt-eval.cc:4025
#8  0x00007f5c551a9e92 in octave::tree_statement_list::accept (tw=..., this=0x7f5c34601b90)
    at libinterp/parse-tree/pt-stmt.h:201
#9  octave::tree_evaluator::execute_user_function (this=this@entry=0x7f5c340058b8, user_function=...,
    nargout=nargout@entry=2, xargs=...) at libinterp/parse-tree/pt-eval.cc:3664
#10 0x00007f5c550c7523 in octave_user_function::execute (args=..., nargout=<optimized out>, tw=..., this=0x7f5c346a0f60)
    at libinterp/octave-value/ov-usr-fcn.cc:495
#11 octave_user_function::call (this=0x7f5c346a0f60, tw=..., nargout=2, args=...)
    at libinterp/octave-value/ov-usr-fcn.cc:488
#12 0x00007f5c551c4984 in octave::tree_index_expression::evaluate_n (this=0x7f5c345720b0, tw=..., nargout=2)
    at libinterp/parse-tree/pt-idx.cc:408
#13 0x00007f5c55194269 in octave::tree_multi_assignment::evaluate_n (this=0x7f5c34600ba0, tw=...)
    at libinterp/parse-tree/pt-assign.cc:201
#14 0x00007f5c55196b34 in octave::tree_multi_assignment::evaluate (this=<optimized out>, tw=..., nargout=<optimized out>)
    at libinterp/parse-tree/pt-assign.h:156
#15 0x00007f5c551b48ad in octave::tree_evaluator::visit_statement (this=0x7f5c340058b8, stmt=...)
    at libinterp/parse-tree/pt-eval.cc:3940
#16 0x00007f5c551a069c in octave::tree_statement::accept (tw=..., this=0x7f5c34602da0)
    at libinterp/parse-tree/pt-stmt.h:124
#17 octave::tree_evaluator::visit_statement_list (this=0x7f5c340058b8, lst=...) at libinterp/parse-tree/pt-eval.cc:4025
#18 0x00007f5c551a9e92 in octave::tree_statement_list::accept (tw=..., this=0x7f5c3459e4b0)
    at libinterp/parse-tree/pt-stmt.h:201
#19 octave::tree_evaluator::execute_user_function (this=this@entry=0x7f5c340058b8, user_function=...,
    nargout=nargout@entry=1, xargs=...) at libinterp/parse-tree/pt-eval.cc:3664
#20 0x00007f5c550c7523 in octave_user_function::execute (args=..., nargout=<optimized out>, tw=..., this=0x7f5c34553e70)
    at libinterp/octave-value/ov-usr-fcn.cc:495
#21 octave_user_function::call (this=0x7f5c34553e70, tw=..., nargout=1, args=...)
    at libinterp/octave-value/ov-usr-fcn.cc:488
#22 0x00007f5c551c4984 in octave::tree_index_expression::evaluate_n (this=0x7f5c345b1b10, tw=..., nargout=1)
    at libinterp/parse-tree/pt-idx.cc:408
#23 0x00007f5c551c5654 in octave::tree_index_expression::evaluate (this=<optimized out>, tw=..., nargout=<optimized out>)
    at libinterp/parse-tree/pt-idx.h:108
#24 0x00007f5c551ac57d in octave::tree_evaluator::convert_to_const_vector (this=this@entry=0x7f5c340058b8,
    args=args@entry=0x7f5c345b1d60) at libinterp/parse-tree/pt-eval.cc:2307
#25 0x00007f5c551c4c04 in octave::tree_index_expression::evaluate_n (this=0x7f5c345b1d90, tw=..., nargout=1)
#26 0x00007f5c551c5654 in octave::tree_index_expression::evaluate (this=<optimized out>, tw=..., nargout=<optimized out>)
    at libinterp/parse-tree/pt-idx.h:108
#27 0x00007f5c551ddeb4 in octave::tree_prefix_expression::evaluate (this=0x7f5c345b2280, tw=...)
    at libinterp/parse-tree/pt-unop.cc:80
#28 0x00007f5c551a8a9c in octave::tree_evaluator::is_logically_true (this=0x7f5c340058b8, expr=0x7f5c345b2280,
    warn_for=0x7f5c557ca045 "if") at libinterp/parse-tree/pt-eval.cc:4539
#29 0x00007f5c551b39ba in octave::tree_evaluator::visit_if_command_list (this=0x7f5c340058b8, lst=...)
    at libinterp/parse-tree/pt-eval.cc:3783
#30 0x00007f5c551b4825 in octave::tree_evaluator::visit_statement (this=0x7f5c340058b8, stmt=...)
    at libinterp/parse-tree/pt-eval.cc:3915
#31 0x00007f5c551a069c in octave::tree_statement::accept (tw=..., this=0x7f5c345b2ba0)
    at libinterp/parse-tree/pt-stmt.h:124
#32 octave::tree_evaluator::visit_statement_list (this=0x7f5c340058b8, lst=...) at libinterp/parse-tree/pt-eval.cc:4025
#33 0x00007f5c551a9e92 in octave::tree_statement_list::accept (tw=..., this=0x7f5c341ed320)
    at libinterp/parse-tree/pt-stmt.h:201
#34 octave::tree_evaluator::execute_user_function (this=this@entry=0x7f5c340058b8, user_function=...,
    nargout=nargout@entry=0, xargs=...) at libinterp/parse-tree/pt-eval.cc:3664
#35 0x00007f5c550c7523 in octave_user_function::execute (args=..., nargout=<optimized out>, tw=..., this=0x7f5c344ddf20)
    at libinterp/octave-value/ov-usr-fcn.cc:495
#36 octave_user_function::call (this=0x7f5c344ddf20, tw=..., nargout=0, args=...)
    at libinterp/octave-value/ov-usr-fcn.cc:488
#37 0x00007f5c551c4984 in octave::tree_index_expression::evaluate_n (this=0x7f5c341a64b0, tw=..., nargout=0)
    at libinterp/parse-tree/pt-idx.cc:408
#38 0x00007f5c551c5654 in octave::tree_index_expression::evaluate (this=<optimized out>, tw=..., nargout=<optimized out>)
    at libinterp/parse-tree/pt-idx.h:108
#39 0x00007f5c551b48ad in octave::tree_evaluator::visit_statement (this=0x7f5c340058b8, stmt=...)
    at libinterp/parse-tree/pt-eval.cc:3940
#40 0x00007f5c551a069c in octave::tree_statement::accept (tw=..., this=0x7f5c344b2540)
    at libinterp/parse-tree/pt-stmt.h:124
#41 octave::tree_evaluator::visit_statement_list (this=0x7f5c340058b8, lst=...) at libinterp/parse-tree/pt-eval.cc:4025
#42 0x00007f5c551a204b in octave::tree_statement_list::accept (tw=..., this=<optimized out>)
    at libinterp/parse-tree/pt-stmt.h:201
#43 octave::tree_evaluator::eval (this=0x7f5c340058b8, stmt_list=..., interactive=<optimized out>)
    at libinterp/parse-tree/pt-eval.cc:985
#44 0x00007f5c551b0973 in octave::tree_evaluator::repl (this=this@entry=0x7f5c340058b8)
    at libinterp/parse-tree/pt-eval.cc:804
#45 0x00007f5c5556b9a1 in octave::interpreter::main_loop (this=this@entry=0x7f5c34004a60)
    at libinterp/corefcn/interpreter.cc:1345
#46 0x00007f5c55574ee8 in octave::interpreter::execute (this=this@entry=0x7f5c34004a60)
    at libinterp/corefcn/interpreter.cc:913
#47 0x00007f5c55d0d882 in octave::interpreter_qobject::execute (this=0x555ef7d82040)
    at libgui/src/interpreter-qobject.cc:89
#48 0x00007f5c51d4a40e in QObject::event(QEvent*) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#49 0x00007f5c5280e713 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
   from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
#50 0x00007f5c55d6495b in octave::octave_qapplication::notify (this=0x555ef7c32db0, receiver=<optimized out>,
    ev=<optimized out>) at libgui/src/octave-qobject.cc:146
#51 0x00007f5c51d1ce2a in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
   from /lib/x86_64-linux-gnu/libQt5Core.so.5
#52 0x00007f5c51d1ff17 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()

Rik <rik5>
Project Administrator
Tue 24 May 2022 03:59:42 PM UTC, comment #16: 

The diff fixes the memory leak issue as verified with the scripts on this bug report.  However, 'make check' fails repeatedly with a segfault in Array.cc-tst.

  liboctave/array/Array.cc-tst ...................................fatal: caught signal Segmentation fault -- stopping myself...

Rik <rik5>
Project Administrator
Tue 24 May 2022 02:56:35 PM UTC, comment #15: 

Yes, memory usage is stable for me too with that change.

Is it the case that without marking that function closure context, its destructor never gets called?

@jwe: when you're ready, could you push the change to stable so 7.2 gets it?

Arun Giridhar <arungiridhar>
Project Member
Tue 24 May 2022 12:59:02 PM UTC, comment #14: 

The attached change appears to avoid the leak for me.

(file #53248)

John W. Eaton <jwe>
Project Administrator
Tue 17 May 2022 09:55:53 PM UTC, comment #13: 

I knew subfunctions were different, but felt it worthwhile to proceed methodically and eliminate possible code paths.

Rik <rik5>
Project Administrator
Tue 17 May 2022 04:52:03 PM UTC, comment #12: 

Subfunctions are completely different from nested functions so any comparison between them isn't really meaningful for this bug report.

I expect this problem is related to exactly what data needs to be stored when returning a handle to a nested function (AKA closure in other languages).  There is probably a circular reference that is not being broken when it should.  See also bug #60237 and the changesets I pushed that I thought fixed it.  I may have missed something.

Other nested function bug reports that might be relevant: bug #60845, bug #60137

John W. Eaton <jwe>
Project Administrator
Mon 16 May 2022 11:12:52 PM UTC, comment #11: 

There are several FIXME notes in ov-fcn-handle.cc and ov-fcn-handle.h pertaining to certain internal functions not handling scope properly. Not sure if related.

Arun Giridhar <arungiridhar>
Project Member
Mon 16 May 2022 10:51:54 PM UTC, comment #10: 

Reviewing the ASAN logs, I think I see the roots of the deadlock, but it is still not 100% clear:

  • An anonymous function (named say f_anon or f_blah) is created. At this point it has made one copy of the input matrix p. It has not been called yet.
  • A nested function calls the anonymous function f_anon which was defined in the enclosing scope. At this point a fresh copy of memory seems to be allocated for that invocation of the anonymous function.
  • When the nested function exits, it does not realize that memory was allocated on its behalf when it invoked the anonymous function, so it does not know to release it. The destructors for those temporary variables are not called.
  • The anonymous function does not know that its caller has exited and therefore does not know to release the memory either.

In effect, any destructors that might be called ordinarily are blocked by the anonymous function waiting for its caller nested function. In C++ terminology, RAII is prevented by the interaction between the nested function and the anonymous function.

Arun Giridhar <arungiridhar>
Project Member
Mon 16 May 2022 10:21:18 PM UTC, comment #9: 

@Rik: I agree with comment #8. It is something to do with "crossing the scope boundary" for lack of a better term. This causes no leak at all:

function test_memleak
  for i = 1:10
    f(rand(1000));
  end
endfunction

function retval = f(p)
  function ret = g()
    ret = (@(x)(1 / rand(1) ./ x))(p);
  endfunction
  retval = g();
endfunction

In that version, the anonymous function is declared inside the subfunction in such a disposable manner that when the subfunction exits it releases all resources.

But if the anonymous function is named outside the subfunction and then used inside it ("crossing the scope boundary"), neither party seems to realize that they ought to release the memory, or that the memory is no longer being used by anyone, so the overhead keeps leaking for every function invocation.

Arun Giridhar <arungiridhar>
Project Member
Mon 16 May 2022 10:16:48 PM UTC, comment #8: 

Observations: This seems to require the interaction of a nested function and an anonymous function within an enclosing ordinary function.

Rik <rik5>
Project Administrator
Mon 16 May 2022 10:13:42 PM UTC, comment #7: 

I also tested using a nested function but avoiding the anonymous function like so

function retval = leak_main_fcn (p)
  R = p*rand(1);

  function retval = leak_nested_fcn (M)

    retval  = leak_sub_fcn (M) ;
  endfunction

  retval = leak_nested_fcn (R);

endfunction

function retval = leak_sub_fcn (M)
  retval  = 1 ./ M;
endfunction

and this also works just fine.

Rik <rik5>
Project Administrator
Mon 16 May 2022 10:08:26 PM UTC, comment #6: 

I tested moving the nested function outside of the main function but still in the same m-file (which makes it a subfunction).  This case works as expected.

function retval = leak_main_fcn (p)
  R = p*rand(1);
  leak_anon_fcn = @() 1./ R;

  retval = leak_sub_fcn (leak_anon_fcn);

endfunction

  function retval = leak_sub_fcn (f)

    retval  = f () ;
  endfunction

Rik <rik5>
Project Administrator
Mon 16 May 2022 09:31:20 PM UTC, comment #5: 

A quick analysis:

  R = p*rand(1);
  leak_anon_fcn = @() 1./ R;

The first line produces one copy of the input in the variable R.

The second line produces a second copy of the input because variables are replaced by their values when an anonymous function is created.  For example,

x = 3;
f = @() 1./ x;

is equivalent internally to

f = @() 1./ 3

I'm not sure where the third copy is coming from.

Rik <rik5>
Project Administrator
Mon 16 May 2022 09:27:04 PM UTC, comment #4: 

I also did some minor reworking of the test case which are attached as memory_leak_script.m

for k = 0:10
  mem = memory();
  printf ("iter: %d, mem_used: %g\n", k, mem.mem_used_octave);
  fflush (stdout);
  leak_main_fcn (rand (4000));
endfor

printf ("------------------------------------------------------\n");
mem = memory();
printf ("iter: %d, mem_used: %g\n", k, mem.mem_used_octave);

and leak_main_fcn.m

function retval = leak_main_fcn (p)
  R = p*rand(1);
  leak_anon_fcn = @() 1./ R;

  function retval = leak_nested_fcn ()
    retval  = leak_anon_fcn () ;
  endfunction

  retval = leak_nested_fcn ();

endfunction

I also confirm increasing memory leakage with each function invocation and that "clear all" does not remedy the situation.

(file #53214, file #53215)

Rik <rik5>
Project Administrator
Mon 16 May 2022 02:17:47 PM UTC, comment #3: 

Attaching ASAN log (mem.txt) for the 7 largest leaks pertaining to this bug. There are indeed three indirect leaks of 8,000,000 bytes per function call (so 3 x 80,000,000 bytes for 10 calls), with a host of many small indirect leaks.

Indirect leak of 80000000 byte(s) in 10 object(s) allocated from:

Indirect leak of 80000000 byte(s) in 10 object(s) allocated from:

Indirect leak of 80000000 byte(s) in 10 object(s) allocated from:

Indirect leak of 3920 byte(s) in 10 object(s) allocated from:

Indirect leak of 3520 byte(s) in 10 object(s) allocated from:

Indirect leak of 2080 byte(s) in 10 object(s) allocated from:

Indirect leak of 1440 byte(s) in 10 object(s) allocated from:

SUMMARY: AddressSanitizer: 240193445 byte(s) leaked in 3502 allocation(s).

(file #53213)

Arun Giridhar <arungiridhar>
Project Member
Mon 16 May 2022 01:06:19 PM UTC, comment #2: 

Was able to quantify it by changing the f(rand(1000)) to other sizes.

For passing a matrix of M bytes to the function f, it leaks 3M bytes + about 500 KB for each call to f().

Thus if the argument to f() is rand(1000) which is 8,000,000 bytes, then it loses just over 24,567,000 bytes per call, Three local copies being made and not being released when the function goes out of scope?

Arun Giridhar <arungiridhar>
Project Member
Mon 16 May 2022 12:42:48 PM UTC, comment #1: 

Can reproduce on on Linux with Octave 8. Changing OS to Any and Release to dev. Listing it as a Segfault since OP got a system freeze from the unchecked leak.

To make it less drastic for testing, here is a milder version that causes memory usage to climb linearly with each execution of test_memleak:

function test_memleak
  for i = 1:10
    f(rand(1000));
  end
endfunction

function retval = f(p)
  R = p * rand(1);
  f_blah = @() 1./R;

  function ret = g()
    ret = f_blah();
  endfunction

  retval = g();
endfunction

Calling "clear all" does not release the memory.

There is NO leak for this version of f(p):

function retval = f(p)
  R = p * rand(1);
  f_blah = @() 1./R;
  retval = f_blah();
endfunction

and this is safe too as expected:

function retval = f(p)
  R = p * rand(1);
  retval = 1./R;
endfunction

If Matlab allows nested functions, could someone please check what it does?

Arun Giridhar <arungiridhar>
Project Member
Sat 14 May 2022 04:34:24 PM UTC, original submission:  

Memory leaks when using nested functions.
Had to shut down and reboot windows when using larger array dimension.  Control-C would not abort script execution.
See example below :

function memory_leak_driver()
  for k = 1:10000
    k
    f(  rand(4000) ) ;
  endfor
endfunction

The misbehaving function "f" is defined as:

  function retval = f( p )
    R = p*rand(1) ;
    f_blah  = @( ) 1./ R ;

    function retval = g()
      retval  = f_blah( ) ;
    endfunction

    retval  = g() ;

endfunction

PIERRE LABRECHE <pierre5018>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #53252:  closure-context-diffs-2.txt added by jwe (999B - text/plain)
file #53248:  closure-context-diffs.txt added by jwe (572B - text/plain)
file #53214:  memory_leak_script.m added by rik5 (298B - text/x-matlab)
file #53215:  leak_main_fcn.m added by rik5 (215B - text/x-matlab)
file #53213:  mem.txt added by arungiridhar (30KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by pierre5018 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only project members can vote.

     

     

     

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-05-24 rik5 StatusPatch Reviewed Fixed
        Open/ClosedOpen Closed
        Releasedev 7.1.0
    2022-05-24 jwe Attached File- Added closure-context-diffs-2.txt, #53252
    2022-05-24 rik5 StatusPatch Submitted Patch Reviewed
    2022-05-24 arungiridhar StatusConfirmed Patch Submitted
    2022-05-24 jwe Attached File- Added closure-context-diffs.txt, #53248
    2022-05-16 rik5 Attached File- Added memory_leak_script.m, #53214
        Attached File- Added leak_main_fcn.m, #53215
    2022-05-16 arungiridhar Attached File- Added mem.txt, #53213
    2022-05-16 arungiridhar Item GroupNone Segfault, Bus Error, etc.
        StatusNone Confirmed
        Release7.1.0 dev
        Operating SystemMicrosoft Windows Any

    Back to the top


    Powered by Savane 3.9