bugGNU Octave - Bugs: bug #53514, Memory leaks in 4.4 release

 
 

bug #53514: Memory leaks in 4.4 release

Submitted by:  Rik <rik5>
Submitted on:  Thu 29 Mar 2018 05:11:24 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  3 - Low Item Group:  Other
Status:  Postponed Assigned to:  None
Originator Name:  Open/Closed:  Closed
Release:  dev Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Mon 03 Sep 2018 01:59:43 AM UTC, comment #17: 

Closing report.  The next memory leak tests will occur before the 5.0 release.

Rik <rik5>
Project Administrator
Tue 03 Apr 2018 11:13:18 PM UTC, comment #16: 

Marking bug as postponed.  The 56B leak is not a showstopper for the 4.4 release.

Rik <rik5>
Project Administrator
Mon 02 Apr 2018 03:34:24 AM UTC, comment #15: 

Is there a way to test for memory leaks over a sustained period of time?  I tried using _run_test_suite_ in a for loop and periodically checking the memory usage of the process with ps.  Unfortunately that did show ever increasing memory usage, but that could be real because of the use of setenv.

Rik <rik5>
Project Administrator
Fri 30 Mar 2018 11:10:16 PM UTC, comment #14: 

I think these small leaks can be left until after the 4.4 release.

John W. Eaton <jwe>
Project Administrator
Fri 30 Mar 2018 05:32:36 AM UTC, comment #13: 

Of course, there is also this comment near register_type

    // Yes, this object is intentionally not deleted in the destructor
    // so that we avoid a crash on exit for user-defined data types.
    // See bug #53156.  If that problem is properly fixed, then this
    // could be stored as an object instead of a pointer to an object
    // allocated with new.

    vals(i) = new octave_value (val);

Rik <rik5>
Project Administrator
Fri 30 Mar 2018 05:28:22 AM UTC, comment #12: 

It seems that it maybe the type_info class needs a destructor which unregisters all the registered types.

The backtrace for one of the 8 byte leaks is

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fe742baf532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x7fe741196ed0 in octave::type_info::register_type(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, octave_value const&, bool) libinterp/octave-value/ov-typeinfo.cc:152
    #2 0x7fe7410c4982 in octave_magic_colon::register_type(octave::type_info&) libinterp/octave-value/ov-colon.cc:34
    #3 0x7fe7411d0b82 in install_types(octave::type_info&) libinterp/octave-value/ov.cc:2932
    #4 0x7fe7411964c8 in octave::type_info::type_info(int) libinterp/octave-value/ov-typeinfo.cc:80
    #5 0x7fe7417cd6d7 in octave::interpreter::interpreter(octave::application*) libinterp/corefcn/interpreter.cc:369
    #6 0x7fe740a9e78b in octave::application::create_interpreter() libinterp/octave.cc:314

interpreter.cc:369 is the constructor for an interpreter object.  One of the data members is m_type_info(). 

The constructor for type_info is next at ov-typeinfo.cc:80 which calls

  install_types (*this);

install_types is in ov.cc and has 55 register_type calls

void
install_types (octave::type_info& ti)
{
  octave_base_value::register_type (ti);
  octave_cell::register_type (ti);
  octave_scalar::register_type (ti);
  octave_complex::register_type (ti);
  octave_matrix::register_type (ti);
  octave_diag_matrix::register_type (ti);
  octave_complex_matrix::register_type (ti);
  octave_complex_diag_matrix::register_type (ti);
  octave_range::register_type (ti);
  octave_bool::register_type (ti);
  octave_bool_matrix::register_type (ti);
  octave_char_matrix_str::register_type (ti);
  octave_char_matrix_sq_str::register_type (ti);
  octave_int8_scalar::register_type (ti);
  octave_int16_scalar::register_type (ti);
  octave_int32_scalar::register_type (ti);
  octave_int64_scalar::register_type (ti);
  octave_uint8_scalar::register_type (ti);
  octave_uint16_scalar::register_type (ti);
  octave_uint32_scalar::register_type (ti);
  octave_uint64_scalar::register_type (ti);
  octave_int8_matrix::register_type (ti);
  octave_int16_matrix::register_type (ti);
  octave_int32_matrix::register_type (ti);
  octave_int64_matrix::register_type (ti);
  octave_uint8_matrix::register_type (ti);
  octave_uint16_matrix::register_type (ti);
  octave_uint32_matrix::register_type (ti);
  octave_uint64_matrix::register_type (ti);
  octave_sparse_bool_matrix::register_type (ti);
  octave_sparse_matrix::register_type (ti);
  octave_sparse_complex_matrix::register_type (ti);
  octave_struct::register_type (ti);
  octave_scalar_struct::register_type (ti);
  octave_class::register_type (ti);
  octave_cs_list::register_type (ti);
  octave_magic_colon::register_type (ti);
  octave_builtin::register_type (ti);
  octave_user_function::register_type (ti);
  octave_dld_function::register_type (ti);
  octave_fcn_handle::register_type (ti);
  octave_fcn_inline::register_type (ti);
  octave_float_scalar::register_type (ti);
  octave_float_complex::register_type (ti);
  octave_float_matrix::register_type (ti);
  octave_float_diag_matrix::register_type (ti);
  octave_float_complex_matrix::register_type (ti);
  octave_float_complex_diag_matrix::register_type (ti);
  octave_perm_matrix::register_type (ti);
  octave_null_matrix::register_type (ti);
  octave_null_str::register_type (ti);
  octave_null_sq_str::register_type (ti);
  octave_lazy_index::register_type (ti);
  octave_oncleanup::register_type (ti);
  octave_java::register_type (ti);
}

If you look at ov-typeinfo.h you find that there is no destructor.

    ~type_info (void) = default;

So, I think what has to happen is that a destructor gets written to call delete for each of the types.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 11:23:42 PM UTC, comment #11: 

Yes, that new is not paired with a delete and it should be.  But I've looked at this recently and it's still not clear to me when that can happen.  I probably could have saved you some time if I'd left a comment there.  Sorry.

John W. Eaton <jwe>
Project Administrator
Thu 29 Mar 2018 08:30:43 PM UTC, comment #10: 

I was able to run through the entire test suite.  I'm attaching the list of "Direct leak"s.  It seems that a fair number are related to octave_putenv which cannot be changed.  56 other instances are related to register_type as I noted in the first submission.  And then there are a few odd leaks that appear to be related to feval.  I don't know if there is a problem with feval, or whether the code that was executed had a leak.

For the 56 instances of register_type, I think there should be a simple solution.  This is code from ov-base.h

  void t::register_type (octave::type_info& ti)                 \
  {                                                             \
    octave_value v (new t ());                                  \
    t_id = ti.register_type (t::t_name, t::c_name, v);          \
  }

It looks to me like that call to new is not paired with a delete.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 06:57:19 PM UTC, comment #9: 

It was just what I thought.  Moving the initialization of variables inside the for loop fixes the problem because the array x is never accessed when it is empty.  See this cset: http://hg.savannah.gnu.org/hgweb/octave/rev/576ff914979c.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 06:39:39 PM UTC, comment #8: 

I'm getting a heap-buffer-overflow in _expint_.cc when running the tests for cosint.m.  Adding Michele Ginesi to the CC list for this bug since he wrote that code.  The abbreviated backtrace from gdb is:

================================================
==18426==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000a952f0 at pc 0x7f0cbc05fef3 bp 0x7f0c960780d0 sp 0x7f0c960780c0
READ of size 16 at 0x602000a952f0 thread T3 (QThread)
    #0 0x7f0cbc05fef2 in F__expint__(octave_value_list const&, int) libinterp/corefcn/__expint__.cc:134
    #1 0x7f0cbbcf6740 in octave_builtin::call(octave::tree_evaluator&, int, octave_value_list const&) libinterp/octave-value/ov-builtin.cc:65
    #2 0x7f0cbc00996c in octave::tree_evaluator::visit_index_expression(octave::tree_index_expression&) libinterp/parse-tree/pt-eval.cc:1335
    #3 0x7f0cbc03cd36 in octave::tree_index_expression::accept(octave::tree_walker&) libinterp/parse-tree/pt-idx.h:102
    #4 0x7f0cbbd78fb7 in octave::tree_evaluator::evaluate(octave::tree_expression*, int) libinterp/parse-tree/pt-eval.h:300

Looking at the code around line 134 in _expint_.cc I see this:

      // Initialize output and inputs
      ComplexColumnVector output (output_dv);
      ComplexNDArray x;

      if (numel_x == 1)
        x = ComplexNDArray (output_dv, args(0).complex_value ());
      else
        x = args(0).complex_array_value ();

      // Initialize variables used in algorithm
      static const Complex tiny = pow (2, -100);
      static const double eps = std::numeric_limits<double>::epsilon ();
      Complex cone (1.0, 0.0);
      Complex czero (0.0, 0.0);
-->   Complex xj = x(0);
      Complex y = tiny;
      Complex Cj = y;
      Complex Dj = czero;
      Complex alpha_j = cone;
      Complex beta_j = xj;
      Complex Deltaj = czero;

My guess is that the code is not properly handling the case where the matrix is empty, i.e., numel == 0.  In that case, we shouldn't be indexing in to the array to get x(0).

The loop below it is

      // Loop over all scenarios
      for (octave_idx_type i = 0; i < numel_x; ++i)
        {
          // Catch Ctrl+C
          OCTAVE_QUIT;

          // Variable initialization for the current element
          xj = x(i);
          y = tiny;
          Cj = y;
          Dj = czero;
          alpha_j = cone;
          beta_j = xj;
          Deltaj = czero;
          j = 1;

Given that the first thing the body of the loop does is to initialize all of these values, it might be better to eliminate the initialization outside the loop.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 06:04:40 PM UTC, comment #7: 

I just configured with '--disable-java' and I no longer get a segfault.  I'm going to leave Java for later.  I'm running 'make check' right now to see if there are any leaks over the entire test suite.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 05:56:13 PM UTC, comment #6: 

I get this:

ASAN:DEADLYSIGNAL
=================================================================
==18437==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fe6d25b02b4 bp 0x7fe6e2c69c00 sp 0x7fe6e458cd88 T6)
==18437==The signal is caused by a READ memory access.
==18437==Hint: address points to the zero page.
    #0 0x7fe6d25b02b3  (<unknown module>)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (<unknown module>)
Thread T6 (QThread) created by T0 here:
    #0 0x7fe71f2e7a2f in pthread_create (/lib64/libasan.so.4+0x37a2f)
    #1 0x7fe719ac3355 in QThread::start(QThread::Priority) (/lib64/libQt5Core.so.5+0xa4355)

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 29 Mar 2018 05:45:23 PM UTC, comment #5: 

I don't get any print out at all when I have leak_check_at_exit=0.  It just runs and exits.

I have -O0 and -ggdb3 in my CFLAGS now, but the backtrace for the segfault in ov-class.cc is still uninformative.

#0  0x00007f52d129f2b4 in  ()
#1  0x49656e696c65746e in  ()
#2  0x02100800000406e3 in  ()
Python Exception <class 'OverflowError'> int too big to convert:
#3  0xbfebfbff7ffafbbf in  ()#4  0x01c0003f1c004121 in  ()
#5  0x000000000000003f in  ()
#6  0x0000000000000000 in  ()

I think this may have something to do with the Java JDK because I get a message in gdb about "Installing openjdk unwinder".  Also, "test ov-java.cc" segfaults in the same way.  I am going to disable java in the build and see what happens.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 05:33:56 PM UTC, comment #4: 

I have to setup this option otherwise the configure step fails at ARPACK detection.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 29 Mar 2018 05:20:34 PM UTC, comment #3: 

@Dmitri: I will try with leak_check_at_exit=0 in about 30 minutes when I have a new build.

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 05:16:36 PM UTC, comment #2: 

Also, something in ov-class.cc is causing a segfault.  I will have to re-compile with debugging enabled to get a backtrace, but for now I have

test ov-class.cc

ASAN:SIGSEGV
=================================================================
==20165==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fde47d022b4 bp 0x7fde584c7260 sp 0x7ffdc57bf3b8 T0)
    #0 0x7fde47d022b3  (<unknown module>)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ??:0 ??
==20165==ABORTING

Rik <rik5>
Project Administrator
Thu 29 Mar 2018 05:15:22 PM UTC, comment #1: 

Do you get those if you set leak_check_at_exit=0 ?

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 29 Mar 2018 05:11:24 PM UTC, original submission:  

I configured with --enable-address-sanitizer-flags and then ran

./run-octave --eval "exit" |& tee leak.log

There is a leak of 8 bytes which is repeated 56 times (approximately the number of octave value types?).  All of the leak records look about like this one:

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7faf538c4532 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99532)
    #1 0x7faf52484e2a in octave::type_info::register_type(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, octave_value const&, bool) (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0xf07e2a)
    #2 0x7faf5231cf6d in octave_magic_colon::register_type(octave::type_info&) (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0xd9ff6d)
    #3 0x7faf524bb5b0 in install_types(octave::type_info&) (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0xf3e5b0)
    #4 0x7faf52484604 in octave::type_info::type_info(int) (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0xf07604)
    #5 0x7faf52d19b53 in octave::interpreter::interpreter(octave::application*) (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0x179cb53)
    #6 0x7faf519eccc7 in octave::application::create_interpreter() (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0x46fcc7)
    #7 0x7faf519ecdee in octave::cli_application::execute() (/home/rik/wip/Projects_Mine/octave-stable/libinterp/.libs/liboctinterp.so.4+0x46fdee)
    #8 0x401748 in main (/home/rik/wip/Projects_Mine/octave-stable/src/.libs/lt-octave-cli+0x401748)
    #9 0x7faf4e91382f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

Rik <rik5>
Project Administrator

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

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
  • -email is unavailable- added by rik5
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by rik5 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-09-03 rik5 Open/ClosedOpen => Closed
    2018-04-03 rik5 Priority5 - Normal => 3 - Low
        StatusIn Progress => Postponed
    2018-03-29 rik5 Carbon-Copy- => Added jwe
    2018-03-29 rik5 StatusNone => In Progress
    2018-03-29 rik5 Carbon-Copy- => Added m_ginesi

    Back to the top


    Powered by Savane 3.5