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.
|
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.
|
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)
|
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).
|
Sat 12 Oct 2013 11:34:28 PM UTC, comment #13:
Oh good, so we have more than one leak path. Sigh
|
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.
|
Sat 12 Oct 2013 08:16:14 PM UTC, comment #11:
Thanks, I'll take a look at it.
|
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)
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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?
|
Thu 10 Oct 2013 02:56:54 AM UTC, original submission:
Along with evalin, reported earlier, there appears to be a leak in eval.
|