bugGNU Octave - Bugs: bug #40231, Memory leaks when parser fails to...

 
 

bug #40231: Memory leaks when parser fails to parse

Submitted by:  Rik <rik5>
Submitted on:  Thu 10 Oct 2013 02:56:54 AM UTC  
 
Category: InterpreterSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Other
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

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 Jump to the original submission)

Thu 17 Oct 2013 04:53:56 PM UTC, comment #17:

All the leaks seem to be plugged which is good news for Octave. Closing report.

Rik <rik5>
Project Administrator
Thu 17 Oct 2013 04:34:23 PM UTC, comment #16:

Thanks for finding this test case. I checked in the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/9f6e4e5c2bac

It seems to fix the problem for me. If you agree, please close this report.

John W. Eaton <jwe>
Project Administrator
Wed 16 Oct 2013 06:45:38 PM UTC, comment #15:

I narrowed down the memory leak in Feval to this test

The resulting valgrind output is

For simplicity, I've attached the bad file as try.tst to this report.

(file #29389)

Rik <rik5>
Project Administrator
Tue 15 Oct 2013 11:58:13 PM UTC, comment #14:

The leak through Feval and sparse was a true leak in the Sparse constructor, not the parser. I plugged it here (http://hg.savannah.gnu.org/hgweb/octave/rev/f4b0430fa5fd).

Rik <rik5>
Project Administrator
Sat 12 Oct 2013 11:34:28 PM UTC, comment #13:

Oh good, so we have more than one leak path. Sigh

Rik <rik5>
Project Administrator
Sat 12 Oct 2013 08:21:57 PM UTC, comment #12:

This doesn't look like the same leak. The one previously was inside the parser, apparently related to octave_base_parser::make_constant.

John W. Eaton <jwe>
Project Administrator
Sat 12 Oct 2013 08:16:14 PM UTC, comment #11:

Thanks, I'll take a look at it.

John W. Eaton <jwe>
Project Administrator
Sat 12 Oct 2013 07:52:53 PM UTC, comment #10:

Attached is a simple one-line program which makes use of %!error which shows a leak through Feval.

where eval_leak.tst is just

(file #29364)

Rik <rik5>
Project Administrator
Sat 12 Oct 2013 07:38:23 PM UTC, comment #9:

Maybe not too painful. Just running the tests for in sparse.tst are enough to uncover one example. Of course, sparse.tst has 1204 tests so bisecting that will still take some time.

I ran Octave under valgrind and then

The leak report is below, and there is a big distance from the Feval to the actual leak which complicates debugging.

Rik <rik5>
Project Administrator
Sat 12 Oct 2013 05:13:24 AM UTC, comment #8:

Valgrind still showed a leak in the parser inside a call to eval when I ran the full set of tests. It's not obvious to me what could cause it and the valgrind output doesn't point to which test(s) is (are) causing it, so I don't know how to debug it. I guess we could bisect the test suite until we find the particular test that generates the leak, but that sounds painful.

John W. Eaton <jwe>
Project Administrator
Sat 12 Oct 2013 04:48:40 AM UTC, comment #7:

Seems to work for the simple cases I tried. Final judgement is the full 'make check' you are running.

Rik <rik5>
Project Administrator
Fri 11 Oct 2013 04:31:52 PM UTC, comment #6:

I also checked in this changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/bd0a84de3375

It should take care of most (all?) other cases in which we need to delete partially constructed parse trees when an error occurs.

I'm running the test suite with valgrind now. If that doesn't show any leaks coming from inside the parser, I'll close this report.

John W. Eaton <jwe>
Project Administrator
Fri 11 Oct 2013 02:16:50 PM UTC, comment #5:

I checked in the following changeset.

http://hg.savannah.gnu.org/hgweb/octave/rev/1e8f8900a041

It seems to avoid the leak for the parse error in eval but not for "if (1) endfor". I thought it might fix both, but apparently not. I'll take another look.

John W. Eaton <jwe>
Project Administrator
Thu 10 Oct 2013 10:29:14 PM UTC, comment #4:

This was never marked as a blocker for the release, but I will re-title the report. The other report, for evalin, I will close since it certainly the same thing.

Rik <rik5>
Project Administrator
Thu 10 Oct 2013 09:06:23 PM UTC, comment #3:

I don't think this problem is specific to switch statements or eval. You can also see a leak if you just do

at the command prompt. We surely need better error recovery in the parser, but I don't think this is a new issue. I don't think it is worth blocking the release for it as I'm sure this bug has been around for as long as Octave.

John W. Eaton <jwe>
Project Administrator
Thu 10 Oct 2013 07:43:31 PM UTC, comment #2:

I got the whole ginormous report after running the test suite under valgrind so I don't have a clear idea which test caused the failure. I seem to remember that switch statements were a problem (see bug #35154).

I just tried the demo code there

and I get a memory leak in eval shown below.

Rik <rik5>
Project Administrator
Thu 10 Oct 2013 06:17:07 PM UTC, comment #1:

I can't seem to duplicate this with a simple "eval ('1')". Do you have any clue about what is being evaluated to trigger this leak?

John W. Eaton <jwe>
Project Administrator
Thu 10 Oct 2013 02:56:54 AM UTC, original submission:

Along with evalin, reported earlier, there appears to be a leak in eval.

Rik <rik5>
Project Administrator

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #29389:  try.tst added by rik5 (148B - application/octet-stream)
file #29364:  eval_leak.tst added by rik5 (33B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5 (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 17 Oct 2013 04:53:56 PM UTCrik5StatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Thu 17 Oct 2013 04:34:23 PM UTCjweStatusConfirmed=>Ready For Test
    Wed 16 Oct 2013 06:45:38 PM UTCrik5Attached File-=>Added try.tst, #29389
    Sat 12 Oct 2013 07:52:53 PM UTCrik5Attached File-=>Added eval_leak.tst, #29364
    Thu 10 Oct 2013 10:29:14 PM UTCrik5StatusNone=>Confirmed
      SummaryMemory leak in eval=>Memory leaks when parser fails to parse

    Back to the top


    Powered by Savane 3.1-cleanup1