bugGNU Octave - Bugs: bug #47372, Memory leaks and segmentation...

 
 

bug #47372: Memory leaks and segmentation faults in Octave

Submitted by:  Rik <rik5>
Submitted on:  Wed 09 Mar 2016 06:34:37 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  Closed
Release:  dev Operating 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)

Fri 13 May 2016 04:58:18 PM UTC, comment #121: 

After 120 comments, and a whole lot of progress (thanks everyone!), I'm going to close this report.

There are still issues with segfaults in the OpenGL code when the address sanitizer option has been compiled in.  I will file a new report to keep track of that.

Rik <rik5>
Project Administrator
Fri 29 Apr 2016 06:13:07 PM UTC, comment #120: 

Also, the crash only happens for me using the address sanitizer build.  If I use a normal build I don't see a crash even when running all the demos.

And, the crash happened when I was running Octave over an ssh connection from a system with an AMD graphics card to a system with an Nvidia card.

When I ran the demos directly on the system with the Nvidia card, I didn't see the crash, even with the address sanitizer build.  This system is using the nouveau driver.

John W. Eaton <jwe>
Project Administrator
Fri 29 Apr 2016 03:34:25 PM UTC, comment #119: 

Rik, RE the crash for

demo ("patch", 2)

I tried an address sanitizer build on a system with an Nvidia graphics card and I can duplicate the crash.  If I make either of the two attached changes, I can avoid the crash.  I'm not sure why the tesselator part doesn't mix with the calls to glVertex3d.

I did some tracing, and for this demo, both branches of the

if (clipf(i))

statement are executed and it looks to me like the calls to vertex3 are correctly inside calls to glBegin and GlEnd.

So I'm a bit lost at this point about why it fails.

At first I thought it might be the way we are initializing OpenGL (not creating buffers correctly or something like that) but it fails for both FLTK and Qt.  So whatever we are doing wrong is wrong for both.

Any clues about how to debug this problem would be most welcome.

Maybe at this point we should close this report and open a separate one for the OpenGL issues or move this info to an existing OpenGL plotting bug report.

John W. Eaton <jwe>
Project Administrator
Wed 13 Apr 2016 02:09:45 AM UTC, comment #118: 

Good catch on the leaked QStringListModel.  I thought that the QListView acted as a container class, but the documentation clearly states that isn't necessarily so (because the design allows for multiple views to display a particular model).  The Qt documentation also states:

"The view does not take ownership of the model unless it is the model's parent object because the model may be shared between many different views."

So maybe the following would have worked too:

   QListView *view = new QListView;
   QAbstractItemModel *model = new QStringListModel (list, view);
   view->setModel (model);

http://doc.qt.io/qt-4.8/objecttrees.html
http://doc.qt.io/qt-5/qstringlistmodel.html

Dan Sebald <sebald>
Tue 12 Apr 2016 09:04:38 PM UTC, comment #117: 

I've updated to the latest tip and have been experimenting some more.  I always get an abrupt failure for

demo ("patch", 2)

This is an "unclosed" patch which I know for certain uses a different code path in gl-render.cc.  It now seems probable that we are using OpenGl slightly incorrectly--not in an obviously incorrect way or it would fail everywhere, all the time.  But on some systems our usage must be unkosher because we do see occasional failures, particularly on systems using Nvidia HW accelerated OpenGL. 

Rik <rik5>
Project Administrator
Tue 12 Apr 2016 02:13:21 PM UTC, comment #116: 

I checked in the following changeset for the ListDialog leak:

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

John W. Eaton <jwe>
Project Administrator
Tue 12 Apr 2016 01:47:28 PM UTC, comment #115: 

I checked in the following changeset for the program name problem.

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

After making this change, I was able to build with OpenGL enabled (but I left docs and java disabled) and run "make check".  No obviously new leaks due to OpenGL showed up for me when doing that.

I'm still unable to start the GUI with the address sanitizer.

I tried running "./run-octave --no-gui" and doing "demo colorbar" and then exiting.  I saw

SUMMARY: AddressSanitizer: 31450 byte(s) leaked in 712 allocation(s).

but nothing else.  I'm using the qt toolkit.

Then I tried running "./run-octave --no-gui" and doing "rundemos" to run all the demos (we have a large collection now!) and was able to step through all of them with no crashes or major issues.  The summary was:

SUMMARY: AddressSanitizer: 89375 byte(s) leaked in 1580 allocation(s).

I'm attaching the full leak log for this test.

In all of that, there is one one case that points directly back to Octave (72 bytes lost in three allocations from the ListDialog object).

Given all that Octave does for those demos, I'd say that's a remarkable result!

(file #36910)

John W. Eaton <jwe>
Project Administrator
Mon 11 Apr 2016 09:16:48 PM UTC, comment #114: 

@Rik (comment #109)

Thanks for your remarks. Actually, the second copy
is not existing in the script, It's only copy/paste error here.

I will make some fixes according to you suggestions.

Avinoam Kalma <avinoam>
Project Member
Fri 08 Apr 2016 09:49:24 PM UTC, comment #113: 

Fun!

John W. Eaton <jwe>
Project Administrator
Fri 08 Apr 2016 09:39:55 PM UTC, comment #112: 

I've been pressing on with the search for leaks.  Up until now I have disabled opengl during configure.  I just enabled it, re-built, and tried "plot (1:10)" which resulted in a use-after-free error from AddressSanitizer and the immediate halt of Octave.  I guess there is more to be done...

In this case, it seems that octave_env::set_program_name is to blame because I commented it out and provided a static string and now it works.

diff -r e3888aa9a4d1 libinterp/octave.cc
--- a/libinterp/octave.cc        Thu Apr 07 15:55:10 2016 -0700
+++ b/libinterp/octave.cc        Fri Apr 08 14:19:14 2016 -0700
@@ -786,7 +786,8 @@ octave_initialize_interpreter (int argc,

   octave_embedded = embedded;

-  octave_env::set_program_name (argv[0]);
+  //octave_env::set_program_name (argv[0]);
+  octave_env::set_program_name ("foobar");

   octave_program_invocation_name = octave_env::get_program_invocation_name ();
   octave_program_name = octave_env::get_program_name ();

With that done, I can now find new, ugly memory violations.

For instance,

demo ("colorbar", 5)

which always causes a heap-buffer-overflow error for me.

Rik <rik5>
Project Administrator
Fri 08 Apr 2016 07:47:17 PM UTC, comment #111: 

It seems okay to ignore the Java leak.  I never saw a size increase over time, which was my main concern, because that could eventually lead to an OOM error.

I still think it would be wise to call terminate_jvm during the shutdown process so that the JVM will finalize any buffers it has.

There are still many threads to complete, but they don't belong to this bug report; Marking as fixed and closing report.

Rik <rik5>
Project Administrator
Fri 08 Apr 2016 06:32:13 PM UTC, comment #110: 

I think the biggest leak for the Java interface comes from loading octave.jar.  It seems that when that is done, the JVM never releases the memory for it.

I'm attaching a simple self-contained program that shows leaks for just loading the jvm shared library, creating a jvm and then destroying it and unloading the shared library.

I think we should just give up on fixing any of these problems.

(file #36868)

John W. Eaton <jwe>
Project Administrator
Fri 08 Apr 2016 05:08:50 PM UTC, comment #109: 

@Avinoam: I do two things slightly differently in my MXE build.

First, are you sure you are not overwriting the tar.gz file with a different one from the MXE directory.

## You are currently in octave-build directory when this is executed.
cp octave-*.tar.gz ~/mxe-octave/pkg
cd ../mxe-octave
hg -v pull
hg -v update
## You are now in mxe-octave directory
## The command below will copy any tar.gz files at the top-level of mxe-octave
## into the pkg directory, overwriting the tar.gz file from step 1.
cp octave-*.tar.gz ~/mxe-octave/pkg

Second, does the stanza

./configure --enable-octave=default
touch src/default-octave.mk

actually prevent MXE from downloading a tar.gz file from the Hydra build server?

I usually explicitly prevent that by applying this patch to default-octave.mk.

diff -r 5ae69991eaa1 src/default-octave.mk
--- a/src/default-octave.mk        Tue Apr 05 17:18:50 2016 -0400
+++ b/src/default-octave.mk        Fri Apr 08 09:57:17 2016 -0700
@@ -7,10 +7,10 @@
PKG             := default-octave
$(PKG)_IGNORE   :=
$(PKG)_VERSION  := 4.1.0+
-$(PKG)_CHECKSUM := ## No checksum
+#$(PKG)_CHECKSUM := ## No checksum
$(PKG)_SUBDIR   := octave-$($(PKG)_VERSION)
$(PKG)_FILE     := octave-$($(PKG)_VERSION).tar.gz
-$(PKG)_URL      := http://hydra.nixos.org/job/gnu/octave-default/tarball/latest/download
+#$(PKG)_URL      := http://hydra.nixos.org/job/gnu/octave-default/tarball/latest/download
ifeq ($(USE_SYSTEM_FONTCONFIG),no)
   $(PKG)_FONTCONFIG := fontconfig
endif

There is no URL specified, and no checksum either, so MXE is forced to use the P(PKG)_FILE octave...tar.gz which is what I want.

Rik <rik5>
Project Administrator
Fri 08 Apr 2016 04:08:21 PM UTC, comment #108: 

Just for the record, I'm attaching the list of leaks that I see when I do

octave --eval "__java_init__; __java_exit__"

Since _java_exit_ is supposed to terminate the jvm and unload the jvm shared library I would expect no leaks.

(file #36866)

John W. Eaton <jwe>
Project Administrator
Fri 08 Apr 2016 04:05:35 PM UTC, comment #107: 

Avinoam: I don't see anything wrong with the way you are rebuilding Octave.

John W. Eaton <jwe>
Project Administrator
Fri 08 Apr 2016 05:12:03 AM UTC, comment #106: 

John: Your version does not segfaults for
"test imread; clear all; test imread",
so the problem is with my build.

I have used the following script to build octave:

hg clone http://hg.savannah.gnu.org/hgweb/octave
cd octave
./bootstrap
cd ..
mkdir octave-build
cd octave-build
../octave/configure
make
make dist
cp octave-*.tar.gz ~/mxe-octave/pkg
cd ../mxe-octave
hg -v pull
hg -v update
cp octave-*.tar.gz ~/mxe-octave/pkg

PATH=`pwd`/usr/bin:$PATH
autoconf
./configure --enable-octave=default
touch src/default-octave.mk

make nsis-installer

Before starting, I have removed old octave and octave-build directories,
so the building was from scratch. I did not remove the old mxe-octave directory.

Avinoam Kalma <avinoam>
Project Member
Thu 07 Apr 2016 08:21:22 PM UTC, comment #105: 

Avinoam: I would rather not upload a build of a random dev version to alpha.gnu.org but I will put it somewhere and let you know privately where you can get it.

When you say "I have build Octave from scratch (hg clone etc...),
but I did did not build the MXE environment from scratch.", what exactly did you do to use the new version that you built?

I'm fairly certain that the bug didn't have anything to do with GraphicsMagick.  It was all about the way static variables are handled on Windows systems.

John W. Eaton <jwe>
Project Administrator
Thu 07 Apr 2016 04:11:24 AM UTC, comment #104: 

@John, could you upload your installer to http://alpha.gnu.org/gnu/octave ?

Avinoam Kalma <avinoam>
Project Member
Thu 07 Apr 2016 04:04:05 AM UTC, comment #103: 

@John,

I have build Octave from scratch (hg clone etc...),
but I did did not build the MXE environment from scratch.
I have also used GraphicsMagick with quantum-depth=16, and not 8.

Could it be the reason for the difference?

Avinoam Kalma <avinoam>
Project Member
Thu 07 Apr 2016 03:43:18 AM UTC, comment #102: 

RE: comment #86, I can run

while (true); test imread; clear all; endwhile

in both octave-cli and octave-gui and neither segfaults.  They just keep printing the warning from GraphicsMagick and the test results.

Avinoam, is it possible you have something left over from a previous build, or that you are somehow using old versions of libinterp and liboctave?

John W. Eaton <jwe>
Project Administrator
Thu 07 Apr 2016 03:18:33 AM UTC, comment #101: 

Rik: I installed the debug symbols for the Qt libraries and single stepped through until the exit happened.

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 09:37:31 PM UTC, comment #100: 

Using gdb with octave-gui I find that the last instruction I can execute is show() at libgui/src/main-window.cc:1113.  This is at the end of

main_window::set_window_layout (QSettings *settings)

This might be because I don't have debug-enabled libraries for Qt.

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 09:35:32 PM UTC, comment #99: 

I saw the same behavior with JVM.  I still think it is the right thing to do to call _java_exit_ before closing Octave, but it does increase the leak.

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 09:18:12 PM UTC, comment #98: 

I noticed that there was no call to terminate the JVM on exit, at least that I could see.  It looks like the function _java_exit_ is supposed to allow one to do that from the Octave command line.  So I did a comparison of

test ov-java.cc
quit

and

test ov-java.cc
__java_exit__
quit

And I see

SUMMARY: AddressSanitizer: 169705 byte(s) leaked in 60 allocation(s).

for the case without _java_exit_ and

SUMMARY: AddressSanitizer: 177464 byte(s) leaked in 82 allocation(s).

for the case with it.

WTF?

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 07:40:14 PM UTC, comment #97: 

When I run octave-gui directly and step through the startup process, I see it exiting the second time memcpy is called at line 965 in TerminalView.cpp.  I have no idea why this is happening.  The address sanitizer provides a replacement for memcpy but looking at that doesn't give me much of a clue.

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 06:31:20 PM UTC, comment #96: 

I've noticed the same problem with the GUI exiting.

I stepped through with the debugger to try to find where and it seemed to be inside some Qt function, but I'm not sure.  I'll need to do some more checking on that.

You can leave it here.

Let's not stop until there are at least 100 comments.  :-/

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 06:25:49 PM UTC, comment #95: 

This bug report is now very long.  The memory leaks portion has mostly been dealt with.  Do we want to keep it open for the segmentation faults with imread on Windows platforms, or should the discussion move back to the original bug report?

Also, the command

./run-octave -f

returns immediately to the prompt for me.  There doesn't appear to be any segfault, but I can't run a binary built with the AddressSanitizer to check if there are leaks in the GUI.  Should I file a separate report for that or continue here?

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 06:15:48 PM UTC, comment #94: 

I ran 'make check >& check.leak.log' and it produces the same report as before.  This is good, it just indicates a difference in libraries available on the target machine.

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 06:10:56 PM UTC, comment #93: 

I've done some more testing concerning the "test imread" connected segfaults under Windows.

To start with my personal highlight: Even with the 4.0.1 release (official Windows installer) I get a difference between using octave.exe and octave-gui.exe!

Here are the full details:

All tests are done under Windows 7. All compilation was done under Ubuntu 14.04.

I used two different tests:

  • test "A" means "test imread, clear all, test imread"
  • test "B" means "test imread, exit"

I have tried three different ways to start Octave:

  • using bin/octave.exe
  • using bin/octave-gui.exe
  • using bin/octave-cli.exe

I have used five different Windows binaries (newest first):

  • "JWEstatic": own mxe-octave build, default branch up to cset ecce63c99c3f, including JWE's patch "avoid multiple definitions of static function-scope vars"
  • "OpenMP": own mxe-octave build, default branch up to cset 6619945e4434, including the "OpenMP patch"
  • "Rikshack": own mxe-octave build, default branch, including Rik's "hack" from comment #12 (that never made it to the repository)
  • "4.0.1": official 4.0.1 release Windows binaries
  • "4.0.0": official 4.0.0 release Windows binaries

My results are:

"JWEstatic" binaries:

  • octave.exe    : A = OK, B = OK
  • octave-gui.exe: A = OK, B = OK
  • octave-cli.exe: A = OK, B = OK

"OpenMP" binaries:

  • octave.exe    : A = OK, B = crash
  • octave-gui.exe: A = OK, B = crash
  • octave-cli.exe: A = OK, B = OK

"Rikshack" binaries:

  • octave.exe    : A = OK, B = OK
  • octave-gui.exe: A = OK, B = OK
  • octave-cli.exe: A = OK, B = OK

"4.0.1" binaries:

  • octave.exe    : A = OK, B = segfault
  • octave-gui.exe: A = crash, B = segfault
  • octave-cli.exe: A = OK, B = OK

"4.0.0" binaries:

  • octave.exe    : A = OK, B = segfault
  • octave-gui.exe: A = OK, B = segfault
  • octave-cli.exe: A = OK, B = OK

Here are some subjective observations on those results:

  • The CLI version never has any problems with this kind of "test imread" segfaults
  • There are many Octave versions, where starting "bin/octave.exe" results in a different segfault behavior compared to "bin/octave-gui.exe". This is even true for official Windows release binaries.
  • The segfault behavior of "bin/octave-gui.exe" changed from the official 4.0.0 release to the official 4.0.1 release.
  • The segfault behavior is different between my Windows PC and Avinoams Windows PC. This difference might either be due to a different Windows environment, or due to a different Linux environment for compilation.

@Avinoam: In order to validate the last point. I would be willing to run your latest Windows executables on my Windows PC if you find a way to make the zip file accessible to me. (I will be two days offline from now on, though...)

I hope that anyone with more understanding can use those results to further hunt down the origin of this bug.

Hartmut <hardy>
Wed 06 Apr 2016 05:35:50 PM UTC, comment #92: 

I had to run "make install" before it would work, but I get the same result if I use the script that calls _run_test_suite_.

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 04:38:26 PM UTC, comment #91: 

I'm just doing "make check >& make-check-log".

This runs (in the build directory):

cd test && $(SHELL) ../run-octave $(RUN_OCTAVE_OPTIONS) --norc --silent --no-history $(abs_top_srcdir)/test/fntests.m $(abs_top_srcdir)/test

In my case, RUN_OCTAVE_OPTIONS is empty.

fntests.m just sets up some options and eventually calls _run_test_suite_, but with some directory arguments.  So I'm not sure what is significantly different.  Looking at the log, about 20k of that is from fontconfig.  About half of that is tagged as "Direct leak" and the other half "Indirect".

I'll try your method and see what I get.  You could try the "make check" test instead and see if it is different for you.

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 04:22:05 PM UTC, comment #90: 

I can confirm that the classdef leaks are now gone.  The only ones I see are now from putenv (which I don't believe can/need to be fixed).

One question, I have a small m-file tst_leak.m that contains

__run_test_suite__
exit

I have been testing by executing this statement from the command line

./run-octave -f tst_leak.m |& tee leak.log

When I do this I find only 3073 bytes are leaked.  However your test log shows 32569 bytes leaked.

How are you running the test suite?  Are you directly starting Octave and entering commands?

./run-octave -f --no-gui
__run_test_suite__
exit

I wasn't overly concerned about the remaining leaks in various parts of the Qt libs because they don't seem to grow with time, and they might just be there because we don't free the resources and instead let the operating system reap the process and all of its memory.  But we could try to locate them.  Shouldn't we be calling QCoreApplication::quit() somewhere?

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 03:01:10 PM UTC, comment #89: 

I checked in the following change for the classdef leaks:

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

The leak report I see now when I run make check with the address sanitizer enabled is attached.  Note that I compiled without Java, so those leaks are omitted from this report.

The remaining leaks directly in Octave all appear to be related to putenv.

I'm not sure how to track down the others since there isn't much information about them.

(file #36845)

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 04:39:55 AM UTC, comment #88: 

Apparently there are many layers to this problem.  If Avinoam's results can be replicated that might indicate a thread race condititon or a problem with the the way execv is working on Windows.

Rik <rik5>
Project Administrator
Wed 06 Apr 2016 04:37:19 AM UTC, comment #87: 

Rik:  I think I understand where the classdef leaks are coming from and I have a preliminary fix.  I think I can come up with a cleaner solution than the one I have now, so I'll work on it a little more before checking in a change.

John W. Eaton <jwe>
Project Administrator
Wed 06 Apr 2016 04:12:05 AM UTC, comment #86: 

Building MXE-Octave from scratch  (changeset 4c61f8313f58, still

test imread; clear all; test imread

produces segmentation fault.

BUT, if I run octave.exe (=octave-4.1.0+.exe) and not octave-gui.exe, I still
get the GUI version, WITHOUT segmentation fault.

Can someone else confirm this?

Avinoam Kalma <avinoam>
Project Member
Tue 05 Apr 2016 06:32:01 PM UTC, comment #85: 

I traced (at least some of) the classdef leaks to the way classdef functions are handled.

The parser generates a classdef object that contains pointers to all the functions (all dynamically allocated memory for the parse tree nodes).  The classdef object is stored in parser.classdef_object.  Then that is used in the parse_fcn_file function to create the meta class for the classdef object, and a pointer to the classdef constructor function is returned.

I think all of the functions end up in the symbol table, but the classdef_object field of the octave_base_parser object never seems to be deleted anywhere that I can see.

John W. Eaton <jwe>
Project Administrator
Tue 05 Apr 2016 04:50:32 PM UTC, comment #84: 

I have taken a preliminary look at the memory leaks through the JNI interface to Java.  First, the interface was coded long ago and doesn't necessarily follow modern JNI practices.  Unless someone has a strong interest, or a financial backer can be found, I think we have to make do with the code we have.

At a minimum, however, I think we ought to be calling terminate_jvm (or use atexit ("__java_exit__") so that we shut down the JVM as we are exiting Octave.  This might prompt the JVM, for example, to write out any buffers to disk, etc.  In any case, I tried the above and it doesn't help with the memory leaks.

The memory leak size seems to be pretty static which is a good thing.  Although it is about 170K on my machine, it doesn't seem to grow if I create and clear javaObjects.  Thus, I don't think it is a threat to a long-running Octave session.  There does seem to be slight growth as new classes are used.  I suspect this is because some information is cached about each class in the JNI interface.

Unless one of the conditions in paragraph 1 is true, I'm going to ignore Java leaks.

Right now (4/5/16) the only remaining leak source to be addressed is the classdef code.

Rik <rik5>
Project Administrator
Mon 04 Apr 2016 04:03:01 PM UTC, comment #83: 

I spoke too soon.  There were more memory leaks in symbfact.cc which uses the CHOLMOD libary.  This was not apparent because there were no BIST tests for this function.  I overhauled symbfact.cc and added BIST tests and plugged the memory leaks (http://hg.savannah.gnu.org/hgweb/octave/rev/bad3ed83330d).

Rik <rik5>
Project Administrator
Mon 04 Apr 2016 04:17:36 AM UTC, comment #82: 

sorry: should be

@Hartmut (comment #79)

Avinoam Kalma <avinoam>
Project Member
Mon 04 Apr 2016 04:13:39 AM UTC, comment #81: 

@Harmut (comment #79)

I have compiled Octave again from changeset d5963a6e29c2 but got the same results
(test imread; clear all; test imread; segfaults)

I did not use the tar.gz from hydra, and I did not build
Octave from scratch, so there might be a difference.

I will check this.

Avinoam Kalma <avinoam>
Project Member
Mon 04 Apr 2016 01:13:46 AM UTC, comment #80: 

It turns out the leak through the cholmod library was Octave's fault after all.  I fixed that here (http://hg.savannah.gnu.org/hgweb/octave/rev/ee1a009dd60f).

As of 4/3/16 this now leaves just two leaks sources:

classdef code (for example: inputParser.m)
Java JVM code (for example: javachk.m)

Rik <rik5>
Project Administrator
Sun 03 Apr 2016 03:59:23 PM UTC, comment #79: 

I have also done a (completly fresh) mxe-octave build of the Octave "default" version (built under Ubuntu 14.04). The Octave sources were used up to cset ecce63c99c3f. This includes jwe's patch to "avoid mulitple definitions of static function-scope vars".

My results under Windows 7 are:

  • "test imread; clear all; test imread" gives NO crash
  • "test imread; exit" gives NO segfault

(I have only tested the GUI versions.)

Everything seems perfect to me! Thanks for this patch, John.

@Avinoam  (and Philip): Could you double check in your mxe-octave file "pkg/octave-4.1.0+.tar.gz" that you have really used the source files INCLUDING jwe's patch? I wasn't able to download a patched tgz file until today in the morning. An easy check is to look at line 208 in "doc/interpreter/splineimage.m" if the semicolon after the demo code has been added (from cset ecc63c99c3f).

Hartmut <hardy>
Sun 03 Apr 2016 10:56:17 AM UTC, comment #78: 

First I want to say "thank you very much" for fixing this long standing issue.
After a _run_test_suite_ and exiting Octave I get no more "post mortem" Windows pop-ups about Octave crashing.  There was no need to disable the evalc tests in .../test/libinterp/parse-tree/oct-parse.in.yy-tst

However, I can confirm Avinoam's observations in comments 76 & 77. But I wonder if that is another issue. "clear all" crashes have been a long-standing issue in esp. MinGW builds.

Philip Nienhuis <philipnienhuis>
Project Member
Sun 03 Apr 2016 04:49:24 AM UTC, comment #77: 

Addition to comment #76:

Only the GUI version segfaults. The CLI does not.

Avinoam Kalma <avinoam>
Project Member
Sun 03 Apr 2016 04:18:09 AM UTC, comment #76: 

checking the sequence

test imread
clear all
test imread

Under Windows (Win-7), it still segfaults.

Avinoam Kalma <avinoam>
Project Member
Fri 01 Apr 2016 07:00:36 PM UTC, comment #75: 

OK, I don't see any additional duplicate static variable definitions with the default branch on Windows.  It is also not crashing on exit after running test imread or the full _run_test_suite_.  I did have to disable the evalc tests because those were causing a crash, but I think that is a separate issue.  I'll try to investigate that and see if I can understand exactly where and why it is crashing.

That was kind of painful but I think this problem is (finally!) fixed now.

Rik, Mike, and everyone else, thanks for helping to fix this long-standing bug.

John W. Eaton <jwe>
Project Administrator
Fri 01 Apr 2016 05:18:12 PM UTC, comment #74: 

The merge was much easier than I expected.  Now building a dev version for Windows to see if there are more fixes required for it.

John W. Eaton <jwe>
Project Administrator
Fri 01 Apr 2016 04:50:06 PM UTC, comment #73: 

I pushed the following changeset:

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

I now see that the empty matrix objects (for example) that are created in .oct files and that previously had low reference counts now have counts in the hundreds, which matches what I see on my GNU system.  Using nm on all the object files for a Windows builds shows no more duplicated guard variables for static variables.  I'm also no longer seeing a crash at exit when running "test imread; exit" or "__run_test_suite__; exit" on Windows.

Now I'll try to merge this with the default branch.

John W. Eaton <jwe>
Project Administrator
Thu 31 Mar 2016 09:39:41 PM UTC, comment #72: 

@jwe: I can see that you are very much on top of this, but--Ugh--it does not look fun.  I'm going back to easy m-files for a while.

Rik <rik5>
Project Administrator
Thu 31 Mar 2016 09:21:27 PM UTC, comment #71: 

Rik: I'm doing as you suggest in comment #70.  I'll have a changeset soon.

I've been finding cases to work on by using nm on all object files and looking for multiple instances of "guard variable for ..." that have the same variable name.  As I understand it, these are the compiler generated global variables that track whether a static variable is initialized.  So if there is only one instance of a guard variable, then I think the static variable should only be initialized once.

c++filt helps with name mangling.

I set KEEP_BUILD := 1 in the mxe-octave Makefile, then I used the following commands in the tmp-stable-octave/octave-4.0.1/.build directory:

$ files=$(find . -name '*.o')
$ for f in $files ; do i686-w64-mingw32-nm $f | i686-w64-mingw32-c++filt | grep 'guard variable' ; done | sort | uniq -c | sort -n > guards

John W. Eaton <jwe>
Project Administrator
Thu 31 Mar 2016 08:32:44 PM UTC, comment #70: 

Seems like the easiest solution is to move the required functions from the header files into the cpp files.  As long as they are small, I would add the inline keyword and also put a comment above the function.  Otherwise I could easily see us cleaning up and accidentally moving these functions back to the header file.

On a separate note, you were concerned about identifying all instances where this might happen.  Can you use the nm utility and check whether a variable is declared 'D' or 'u' between the Linux and Windows binaries to find the differences?  That might be a fairly easy way to get an automatic check, unless the C++ compiler name mangling is too much.

Rik <rik5>
Project Administrator
Thu 31 Mar 2016 06:40:22 PM UTC, comment #69: 

Visibility attributes will help, but that's not the real problem here.

The problem is whether a static symbol is uniquely defined.

After doing some more testing, I think the following are true:

If you have something like the following function defined in a header file that is included in multiple source files, then on Windows systems you'll see multiple definitions of the static variable, one per source file.  And then initialization of that (unshared) "static" variable happen mulitple times.

class dim_vector {
  ...
  static octave_idx_type *nil_rep (void)
  {
    static dim_vector zv (0, 0);
    return zv.rep;
  }
  ...
};

If the definition is moved to a source file, leaving only the declaration in the header file, then you'll get only one copy and a single initialization.

This is different from what happens on (modern?, some?) Unixy systems where only one copy of the static variable is created (or at least, used).

You can see the difference with nm.  On Windows systems, each instance of the static variable has storage class "D" (The symbol is in the initialized data section.) and on my GNU system I see "u" (The symbol is a unique global symbol.  This is a GNU extension to the standard set of ELF symbol bindings.  For such a symbol the dynamic linker will make sure that in the entire process there is just one symbol with this name and type in use.)

Given that this behavior might be GNU-specific, maybe it would be best to move these function definitions to

Another solution appears to be defining static class data members instead of function-scope static variables.  The advantage of doing that is it allows the nil_rep function to be inlined.  The disadvantage is that it moves the static variable outside of the function, and (with current C++ rules) it must be initialized separately.  It seems simpler to just move these functions to source files.

There is an additional problem with implicit instantiation of templates leading to multiple definitions of function-scope static variables.

I don't see how visibility attributes can help with these problems.  For MinGW, we are not using visibility attributes (everything is exported) and there were no warnings about the static variables being defined multiple times.

John W. Eaton <jwe>
Project Administrator
Wed 30 Mar 2016 10:40:34 PM UTC, comment #68: 

According to things I read on the web, if we ever start implementing the visibility attribute for compilation on the Linux side then we should be able to detect these problems on Linux.  That might be a help since the full compile cycle through MXE-Octave is quite long just to test a hypothesis.

Rik <rik5>
Project Administrator
Wed 30 Mar 2016 09:58:11 PM UTC, comment #67: 

OK, I think I have a solution.

Yes, it seems that to export the static data, it needs to be a data member, not just a static variable in a member function.  If it is a static data member and the class is exported, then the data member will also be exported.

Then the other problem was that in addition to Array-d.cc (where Array<double> is explicitly instantiated) we were also getting implicit instantiations in ov-classdef.cc and Array-tc.cc.  Apparently there were Array<double> values referenced in those files and since the complete class definition was available the compiler did its thing and instantiated an instance of Array<double>.

Fixing these problems so that only a single instance of Array<double> is present seems to fix the problem, but I'm sure we have other similar cases to fix.  A few will be easy to find because they follow a similar pattern to this.  But for others I'm guesing it will be a little more difficult.  Maybe we could look at the output of nm on the .o files for Windows and find where data objects are defined more than once and start with those?  I'm not sure.

I'll check in a changeset soon.

John W. Eaton <jwe>
Project Administrator
Wed 30 Mar 2016 06:07:41 PM UTC, comment #66: 

Rik: I just put the verbatim text in the wrong spot.  The text that was supposed to be after "Changing the nil_rep function to do this instead:" was the diff that appeared at the very end of comment #62.

John W. Eaton <jwe>
Project Administrator
Wed 30 Mar 2016 05:48:52 PM UTC, comment #65: 

Even though the discussion went into a different direction. I have done a mxe-octave Windows built with the "default" branch of the Octave repository, using changes up to cset 6619945e4434, this includes the "OpenMP patch".

The results under Windows 7 are:

  • running "test imread, clear all, test imread" does NOT crash any more.
  • running "test imread, exit" still DOES trigger a segfault.

So this OpenMP patch does improve the situation under Windows. But it's not the full solution.

(I haven't succeeded to build a patched Windows version of the "stable" Octave branch, and no give up.)

Hartmut <hardy>
Wed 30 Mar 2016 05:13:03 PM UTC, comment #64: 
Rik <rik5>
Project Administrator
Wed 30 Mar 2016 05:03:37 PM UTC, comment #63: 

I think there may have been a verbatim block missing after "Changing the nil_rep function to do this instead:".  Regardless, I get the idea.  It sure is a pain to develop for Windows.

I think your intuition is correct.  The usual method for exporting symbols in Windows is __declspec.  You might try this thread http://stackoverflow.com/questions/225432/export-all-symbols-when-creating-a-dll#226017.

However, it seems that Windows may not allow non-public data members to be exported.  You can see if you understand the documentation better than I do (https://msdn.microsoft.com/en-us/library/a90k134d.aspx).  One quick test would be to use your last attempt and put nil_rep in as a public variable rather than a private one.

This thread also looked germane: http://stackoverflow.com/questions/19373061/what-happens-to-global-and-static-variables-in-a-shared-library-when-it-is-dynam.  It appears that each thread may get its own copy of a static variable.

In the case of Unix-like environments (like Linux), the dynamic libraries, called "shared objects" with extension .so export all extern global variables (or functions). In this case, if you do load-time linking from anywhere to a shared object file, then the global variables are shared, i.e., linked together as one. Basically, Unix-like systems are designed to make it so that there is virtually no difference between linking with a static or a dynamic library. Again, ODR applies across the board: an extern global variable will be shared across modules, meaning that it should have only one definition across all the modules loaded.

That seems to be what is happening on Linux, and what definitely does not happen on Windows.

Rik <rik5>
Project Administrator
Wed 30 Mar 2016 04:02:36 AM UTC, comment #62: 

I think I've found the reason for the crash on Windows systems.  The problem is not the GraphicsMagick library, but the way dynamic libraries resolve symbols on Windows systems.

In F__magick_formats__, we have

  if (! coder.isReadable ())
    fmt.setfield ("read",  Matrix ());

The empty Matrix object ultimately uses this code from Array<T>:

  typename Array<T>::ArrayRep *nil_rep (void) const
  {
    static typename Array<T>::ArrayRep nr;
    return &nr;
  }

On ELF systems when the main program and its shared libraries are linked with -rdynamic, symbols from shared libraries that are loaded with dlopen are found in the main program and its libraries first, so F__magick_formats__ and Octave both use the same static NR object.  The struct that _magick_foramts_ returns is stored in Octave's symbol table.  If _magick_read_.oct (the shared library where F__magick_formats__ is defined) is dlclosed and unloaded before Octave cleans up the symbol table, there is no problem because the memory for the static NR object is stored in libobctave, which is still loaded.

But on Windows systems, the NR object is that F__magic_formats__ is using is being initialized separately, because when _magick_read_.oct is loaded, the symbols from liboctave are initialized again.

You can easily see this difference if you change the code in F__magick_formats__ to be

  Matrix mtmx;
  if (! coder.isReadable ())
    fmt.setfield ("read",  mtmx);

and then examine the reference count for the rep object.  On my Debian system, I see a count of about 300 when I call _magick_formats_ just after starting Octave.  On Windows, it is just 2.

So the static NR symbol is not from Octave, but from the _magick_read_.oct DLL.  When that file is closed, the pointer to NR is no longer valid.  But it is still stored in Octave in the structure that _magick_formats_ returned, so when Octave exits and the symbol table is cleaned up, Octave crashes when it tries to dereference rep in the expression "if (rep->count--) ..." in the Array destructor.

Changing the nil_rep function to do this instead:

"works".  But that's not really a general solution here, because there may be cases where we actually must access a global variable from liboctave or libinterp inside a DLL.  So what is the proper way to tell the Windows dynamic linker to do this instead of binding to a second copy and/or reinitializing the global state?

Guessing that the problem is that NR is not exported, I tried the following change and Octave still crashed on exit, though in a slightly different location.

diff --git a/liboctave/array/Array.cc b/liboctave/array/Array.cc
--- a/liboctave/array/Array.cc
+++ b/liboctave/array/Array.cc
@@ -2770,6 +2770,10 @@ void Array<T>::instantiation_guard ()
   T::__xXxXx__ ();
}

+template <class T>
+typename Array<T>::ArrayRep
+Array<T>::nil_rep_obj;
+
#define INSTANTIATE_ARRAY(T, API) \
   template <> void Array<T>::instantiation_guard () { } \
   template class API Array<T>
diff --git a/liboctave/array/Array.h b/liboctave/array/Array.h
--- a/liboctave/array/Array.h
+++ b/liboctave/array/Array.h
@@ -149,14 +149,15 @@ protected:

private:

+  static typename Array<T>::ArrayRep nil_rep_obj;
+
   typename Array<T>::ArrayRep *nil_rep (void) const
   {
     // NR was originally allocated with new, but that does not seem
     // to be necessary since it will never be deleted.  So just use
     // a static object instead.

-    static typename Array<T>::ArrayRep nr;
-    return &nr;
+    return &nil_rep_obj;
   }

protected:

John W. Eaton <jwe>
Project Administrator
Tue 29 Mar 2016 04:08:40 PM UTC, comment #61: 

I added a comment to the code to explain why Octave needs this seemingly unneeded piece of initialization (http://hg.savannah.gnu.org/hgweb/octave/rev/f6663c49870c).

Interestingly, libgomp is not linked in to octave-gui even with the new call to omp_get_num_threads (checked with ldd).  But, it no longer segfaults for me.

I fixed another bug on Windows systems (bug #47524) and Avinoam is going to test the fix for that bug and this one.  Fingers crossed.

Rik <rik5>
Project Administrator
Tue 29 Mar 2016 06:20:41 AM UTC, comment #60: 

The change jwe made on stable works for me (rebuilt the default branch with --as-needed). I can confirm with readelf that none of the executables nor shared libraries have dependencies on libgomp.so.1 except for liboctinterp.so now. Running "test imread" doesn't segfault for me, either with octave or octave-cli, looks good.

Mike Miller <mtmiller>
Project Administrator
Tue 29 Mar 2016 01:13:56 AM UTC, comment #59: 

Seems fine to me.  I can make the change on stable and merge with default.

John W. Eaton <jwe>
Project Administrator
Tue 29 Mar 2016 12:48:59 AM UTC, comment #58: 

I really don't like the idea of messing around with the linker flags to solve this one. Other compilers support OpenMP with their own runtimes, I think calling an OpenMP function early in the program to ensure the necessary libraries are linked in as early as possible and never get unloaded is the safer way.

Mike Miller <mtmiller>
Project Administrator
Tue 29 Mar 2016 12:43:16 AM UTC, comment #57: 

So, what should we do, fiddle with linker options (seems fragile, may depend on GNU ld, etc.) or just call an openmp function early on in the Octave initialization (seems simple, but a little bit of a kluge)?

John W. Eaton <jwe>
Project Administrator
Tue 29 Mar 2016 12:39:35 AM UTC, comment #56: 

This is a peculiarity of the way Ubuntu configure's its gcc slightly differently from Debian.

I am able to reproduce your segfault if I edit the libtool script by hand after running configure and force "-Wl,--as-needed" to be present on the link command line very early on, right after where "-nostdlib" appears in my libtool script. Building all of Octave with this option enabled shows the same segfault you've been seeing all along and we on Debian haven't.

I doubt this has any effect on bug #41699, because it looks like the mxe gcc is built with OpenMP disabled, and so is GraphicsMagick.

Mike Miller <mtmiller>
Project Administrator
Tue 29 Mar 2016 12:19:43 AM UTC, comment #55: 

I tested the example on Stack Overflow and it does segfault for me, but not reliably; Sometimes it works, sometimes it doesn't.

When I use ldd on the created main binary I see that libgomp is still not included.  On the other hand, if I disable the linker's automatic pruning of unused libraries it does work.  I only want to disable pruning for this one particular library so I used

setenv LDFLAGS -Wl,--no-as-needed,-lgomp,--as-needed

This fixes things in the test program.

I went back and added the same stanza to the extra LDFLAGS in configure.ac and now it works!  There is no segfault and ldd reports that libgomp is linked in.  This obviously is a GCC version/OS dependent issue.  For the moment, I force GCC to link in libgomp if OpenMP is enabled.  See cset http://hg.savannah.gnu.org/hgweb/octave/rev/f80b46f7d3d8.

The WARN_LDFLAGS variable does not exist on the stable branch so if we want to backport this to the stable branch it will need a different strategy.

Rik <rik5>
Project Administrator
Tue 29 Mar 2016 12:01:11 AM UTC, comment #54: 

Rik, if you try the following patch, is libgomp linked with libinterp?  And if it is, do you still see the segfault?

(file #36787)

John W. Eaton <jwe>
Project Administrator
Mon 28 Mar 2016 11:13:29 PM UTC, comment #53: 

I see libgomp linked to octave-gui and octave-cli.  I used ldd on those files in the src/.libs directory.

No idea yet if this makes any difference on Windows systems (I'm thinking no at this point).

John W. Eaton <jwe>
Project Administrator
Mon 28 Mar 2016 10:58:06 PM UTC, comment #52: 

I tested by adding '-fopenmp' to CFLAGS, CXXFLAGS, and FFLAGS and declaring LDFLAGS to be '-lgomp'.  After that, 'make maintainer-clean" followed by bootstrap, configure, make cycle.  I used V=1 and I see -fopenmp with the compiles and -lgomp with the links.  I still get a segfault.  This is with gcc 5.2.1 which is one revision back from jwe and Mike.

I tried to use ldd to check whether it is linked in, but I'm not sure I'm getting it right.

I used:

cd src/.libs
ldd octave-gui | grep gomp
ldd lt-octave-gui | grep gomp

Neither of these returns anything so it doesn't appear that libgomp is linked in.

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 10:17:13 PM UTC, comment #51: 

With patch for GraphicsMagick applied, remaining leaks as of 3/28/16:

in m-files:

leaks in general/inputParser.m from classdef code
leaks in java/ directory due to JVM

in C++ files:
dldfcn/chol.cc : leaks are within libcholmod in cholmod_malloc and cholmod_realloc.  Probably not Octave's fault.

in fixed tests:
bug-44940/bug-44940.tst : classdef leak
classdef/classdef.tst : classdef leak

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 09:57:05 PM UTC, comment #50: 

Or, can you try running ldd on your octave-gui executable?  For mine, it is already linked with libgomp.  But I'm using gcc 5.3.1.  If you are using an older version, maybe the behavior is different.  I don't think we explicitly add that library to the link flags.

John W. Eaton <jwe>
Project Administrator
Mon 28 Mar 2016 09:53:44 PM UTC, comment #49: 

Rik, can you try adding -lgomp to the link command for Octave as described in the stackoverflow thread?  It probably needs to be added to the octave-gui link command, or possibly libinterp would work?  It may also require the use of the -fopenmp compiler option.

John W. Eaton <jwe>
Project Administrator
Mon 28 Mar 2016 09:48:52 PM UTC, comment #48: 
John W. Eaton <jwe>
Project Administrator
Mon 28 Mar 2016 09:26:31 PM UTC, comment #47: 

You always get a segfault if you use --enable-address-sanitizer-flags, but I haven't seen it yet on my system with --enable-address-sanitizer-flags (with gcc 5.3.1, GraphicsMagick 1.3.23). Maybe I'll try an older system to see if I can reproduce there.

Mike Miller <mtmiller>
Project Administrator
Mon 28 Mar 2016 08:29:37 PM UTC, comment #46: 

Finally, attached is my hack to disable dlclose on the GraphicsMagick library; It stops the segfault for me.  Narrowing the bug down to this particular code path has taken a very long time.  While the correct thing may be to go over to the GraphicsMagick code and start hunting there, I don't have the energy or time to learn someone else's code tree.  Unless someone wants to do that, or there are great ructions about the "hackiness" of this patch, I will apply it this weekend.

(file #36786)

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 08:25:17 PM UTC, comment #45: 

The test bed isn't foolproof.  It seems to manifest always on Windows systems.  For Linux systems, I always get it if I use --enable-sanitizer-flags, but never get it if I don't have that enabled.  To me, that points to it being a true segmentation violation since it seems to depend on how the actual parts of the program are loaded into memory.

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 07:11:02 PM UTC, comment #44: 

I don't think I'm getting the same results that you are. I have built the default branch with

--disable-java --without-opengl --without-qt --enable-address-sanitizer-flags --enable-bounds-check CFLAGS="-O0 -ggdb3" CXXFLAGS="-O0 -ggdb3" FFLAGS="-O0 -ggdb3"

and I get no segfault when I do

./run-octave
octave:1> test imread
PASSES 5 out of 5 tests
octave:2> quit

Is there something else you are doing to provoke the segfault you are seeing from the GraphicsMagick-linked code? Any ASAN_OPTIONS or LSAN_OPTIONS in your environment?

When I run this in gdb I do indeed see threads starting up when "test imread" runs, but exiting shows no segfault. And as I reported earlier, I'm able to run a full "make check" with no segfaults, either during or on exit. Running again now to confirm that it still completes for me.

Mike Miller <mtmiller>
Project Administrator
Mon 28 Mar 2016 06:36:46 PM UTC, comment #43: 

Getting closer...  There is no segfault if I do

setenv OMP_NUM_THREADS 1

before executing run-octave.  On the other hand, if I bump the thread number to 2, or don't set the environment variable, then I do get a segfault.

When the number of threads is 1 I get an 8 byte memory leak so I think there must be something within the GraphicsMagick library and the interaction with OMP.

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 05:20:44 PM UTC, comment #42: 

It's pretty hard to determine, but it appears that this may be a GraphicsMagick library bug, rather than an Octave bug.

If that is the case then I can hack, really hack, a workaround which avoids dlclose if the library is GraphicsMagick.  I'd like to continue to use the dlclose code where possible since it is the right thing to do, and might reveal other problems.

Rik <rik5>
Project Administrator
Mon 28 Mar 2016 10:29:39 AM UTC, comment #41: 

I need to correct my statement from comment #16 where I tested Rik's patch from comment #12 to eliminate the image related crashes under Windows: This patch also DOES work fine with the current "stable" branch of the Octave repo. All my test cases A to D are well behaved. (My formerly observed re-occurence of bug #45372 in test case D was only caused by me, not properly updating the Octave sources for this mxe-octave compilation.)

So now I would conclude that Rik's patch from comment #16 eliminates the mentioned (probably memory leak induced) crashes and segfaults in both, the Octave "default" branch as well as in the "stable" branch.

Should an Octave release 4.0.2 be ever prepared, then I would now vote for this patch to be included there. Sorry for being a bit late with this corrected information.

Hartmut <hardy>
Fri 25 Mar 2016 06:50:32 PM UTC, comment #40: 

I have now tested Rik's patch from comment #12 under Windows 7, this time building mxe-octave with the current DEFAULT branch of Octave (not the stable branch as before).

I did the very same tests as explained in my comment #16. The results are now:

  • A) test imread, clear all, test imread ==> no crash
  • B) test imread, exit ==> no segfault
  • C) playing around with some image processing scripts ==> no segfaults on exit
  • D) displaying an rgb uint8 image and clicking on it (bug #45372) ==> no crash

So this patch DOES remove some long standing crash and segfault bugs under Windows. And I haven't found any side effects under Windows so far.

Hartmut <hardy>
Fri 25 Mar 2016 03:30:11 PM UTC, comment #39: 

When I was working on bug #41699, one of my theories was that GraphicsMagicks builds itself with OpenMP, and when the shared library was released it left an OpenMP thread pool running. Building Octave with -fopenmp (which we do by default now) solved that for me on Debian but didn't solve the crash on Windows, though.

The idea that GraphicsMagick has some kind of resource that isn't released properly when the shared library is unloaded seems plausible to me, whether it's OpenMP threads or static variables or some other resource.

Mike Miller <mtmiller>
Project Administrator
Thu 24 Mar 2016 09:51:42 PM UTC, comment #38: 

Still no luck.  I downloaded and installed GraphicsMagick 1.3.23 from source and re-built Octave and the segfault is still there.

With the instrumented Octave I ran 'test XXX.cc; exit' individually for each of the .cc files in dldfcn.  The only segfaults are related to GraphicsMagick or to DRI in OSMesa.  I did find another leak in chol.cc that I need to investigate.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 08:45:21 PM UTC, comment #37: 

If only _magick_read_ and after looking at the GraphicsMagick changelog entries from 1.3.21 and 1.3.23, then I wouldn't be surprised to find the problem is there, not Octave.

http://hg.code.sf.net/p/graphicsmagick/code/file/50ef32ccc258/ChangeLog.2015

There are many fixes for memory leaks, fixed-size buffers and similar issues.

It looks like we are still using 1.3.21 for our Windows build as well.

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 08:28:48 PM UTC, comment #36: 

Can you make it happen with any other .oct files, or is it just _magick_read_ that has this problem?

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 08:11:06 PM UTC, comment #35: 

Good news / Bad news.  First, the good news is that neither of the two patches fixed the segfault.  This means the nil_rep class variable is probably a red herring.  In another test, I edited _magic_read_.cc and replaced all static variables with ordinary ones.  This also did not fix the problem.

From gdb, the commonality is that all segfaults go through the destructor octave_dlopen_shlib::~octave_dlopen_shlib (void) which calls 'dlclose (library)'.

The bad news is that I don't know where to go from here.  We could remove the dlclose and simply wait until Octave exits and the OS reaps the memory.  Or we could assume that it is some interaction between my version of gcc, glibc (since dlopen/dlclose are used), and GraphicsMagick.

It does segfault reliabliy with the Address Sanitizer turned on which means there is at least a testbed for ideas, but I don't know what they would be.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 07:13:48 PM UTC, comment #34: 

Do you mean that static variables in the shared library that is loaded are causing trouble, or is it the static nil_rep variable that you think is causing the trouble?

If you run the test that causes a crash under gdb, do you get any useful info about where the crash is actually happening?

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 07:12:20 PM UTC, comment #33: 

Do either of the attached patches avoid the problem for you?

(file #36754, file #36755)

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 06:40:57 PM UTC, comment #32: 

It seems like static variables and dynamic shared libraries just done't mix very well.  I don't know whether it is on the GraphicsMagick side, or on our side, but here are some examples of this going badly:

http://www.codeproject.com/Articles/13964/Static-variables-in-shared-libraries

http://stackoverflow.com/questions/2631918/main-program-and-shared-library-initializes-same-static-variable-in-static-ini

http://compgroups.net/comp.lang.c++.moderated/global-static-destructors-called-when/363808

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 06:38:45 PM UTC, comment #31: 

People

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 06:38:02 PM UTC, comment #30: 

test __magic_read__.cc
exit

doesn't produce a segfault so I don't think it is a good test candidate.

_magick_read_ wants a filename and an options structure as inputs.

Instead I tried

while (true)
   test imread
   clear all
endwhile

This happily runs without segfaulting.  However, when I use Ctrl+C to stop the loop and then use 'exit' it segfaults there just as before.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 05:58:08 PM UTC, comment #29: 

Does something like

while true
  __magick_read__  ## or test __magick_read__.cc ?
  clear all
end

trigger a crash for you?

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 05:45:40 PM UTC, comment #28: 

Sorry, GraphicsMagick is what I'm using as well.

The segfault is clearly related to the shared library.  I can "fix" it with the original hack which gets rid of 'delete rep'.  I can also "fix" it by adding RTLD_NODELETE to the original dlopen call.

diff -r ffbd3e86e0be liboctave/util/oct-shlib.cc
--- a/liboctave/util/oct-shlib.cc       Tue Mar 22 23:14:20 2016 -0700
+++ b/liboctave/util/oct-shlib.cc       Thu Mar 24 10:37:24 2016 -0700
@@ -201,6 +201,8 @@ octave_dlopen_shlib::octave_dlopen_shlib
   flags |= RTLD_GLOBAL;
#  endif

+  flags |= RTLD_NODELETE;
+
   library = dlopen (file.c_str (), flags);

   if (! library)

According to the man page for dlopen

     RTLD_NODELETE (since glibc 2.2)
              Do not unload the library during dlclose().  Consequently, the library's static variables are not reinitialized if the
              library is reloaded with dlopen() at a later time.  This flag is not specified in POSIX.1-2001.

In either case, I'm preventing the library from being unloaded.  Still not clear why it is happening though.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 05:07:22 PM UTC, comment #27: 

I'm using GraphicsMagick, not ImageMagick.  Are they the same now?  I'm pretty sure they used to be different.

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 05:04:10 PM UTC, comment #26: 

I ran "make check" again in my address-sanitizer build and I'm not seeing the crash in the QR tests now.  That's without the patch.

The static nil_rep should be OK because shlib_rep's constructor sets count to 1.  So when the static object is first created it has a count of 1.  Then it is incremented in the constructor.  So the count should never go to zero unless an octave_shlib object that is created this way is somehow deleted more than once.  Is that possibly happening?  The path I see that could allow that is something like

  octave_shlib *ptr = new octave_shlib;
  ...
  delete ptr;
  delete ptr;

John W. Eaton <jwe>
Project Administrator
Thu 24 Mar 2016 04:59:10 PM UTC, comment #25: 

The ImageMagick segfault seems to have been much more closely studied in bug #41699.  gdb, along with the address sanitizer, are reporting the bug in dlclose.  But it seems that it is more likely to be due to the ImageMagick library and our implementation of it in _magick_read_.cc.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 04:21:16 PM UTC, comment #24: 

@jwe: Several questions and answers for you.

1) Does the shlib patch work to stop the segfault in qr that you are seeing?

2) You asked for version numbers, I am using Kubuntu 15.10 with ImageMagick libraries 1.3.21-3.  This looks nearly the same as the upstream Debian libraries you report as 1.3.23-1+b1.  I will be upgrading to the 16.04 LTS release (April 21, 2016).  We can always wait until then to see if pulling down newer libraries makes a difference.

3) There still seems to be something dodgy with oct-shlib.  I can't quite put my finger on it, but the coding makes me uneasy.  The code segment below is from oct-shlib.h.  It appears to me that there is a path where delete is not paired with new. 

private:

  static shlib_rep nil_rep;

public:

  octave_shlib (void) : rep (&nil_rep) { rep->count++; }

  octave_shlib (const std::string& f, bool fake = true)
    : rep (shlib_rep::get_instance (f, fake)) { }

  ~octave_shlib (void)
  {
    if (--rep->count == 0)
      delete rep;
  }

If I create a null instance of octave_shlib by calling the constructor with no arguments then rep will point to the static class variable nil_rep, rather than to a heap-allocated rep created through the new operator.  If the destructor is then called and delete is used on the class variable then much mayhem will ensue.

4) Stepping through the debugger, the _init_gnuplot_.oct is successfully freed.  The segfault happens with _magick_read_.oct and the last code that I can execute is in octave_dlopen_shlib::~octave_dlopen_shlib which calls dlclose (library).  It's the dlclose that results in a segfault.

5) The summary of open issues from 3/13 remains the same for me.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 03:26:57 PM UTC, comment #23: 

@Hartmut: I'm also voting for testing the patch with the dev branch.  The stable branch has just been released as 4.0.1 so we're not going to put the patch there anymore.

Rik <rik5>
Project Administrator
Thu 24 Mar 2016 12:35:49 PM UTC, comment #22: 

@Hartmut (comment #16)

I have tested the patch from comment #12 under Windows, with
Octave version from the development branch (not the stable)
I saw (A) & (B) (no segfaults), but I could not
reproduce (D)  - no crash with your examples.

Could you please check the patch also with the dev branch?

Avinoam Kalma <avinoam>
Project Member
Sat 19 Mar 2016 01:41:57 AM UTC, comment #21: 

And with my latest changes to textscan.cc I can now run a full make check on default with no patches applied without a segfault with the AddressSanitizer in use. Very long leak report, but no segfaults.

Mike Miller <mtmiller>
Project Administrator
Fri 18 Mar 2016 08:05:44 AM UTC, comment #20: 

I built with

--disable-java \
--without-opengl \
--without-qt \
--enable-address-sanitizer-flags \
CFLAGS="-O0 -ggdb3" \
CXXFLAGS="-O0 -ggdb3" \
FFLAGS="-O0 -ggdb3"

I don't get any segfaults from a full make check, or by individually testing imread, imwrite, or qr. I do get a segfault in the new textscan.cc, but I cleared the file textscan.cc-tst to bypass this new code and the rest of the test suite runs fine. I do get a long report about memory leaks.

Mike Miller <mtmiller>
Project Administrator
Fri 18 Mar 2016 01:32:19 AM UTC, comment #19: 

I've updated to the latest code and I get segfaults with 'test imread' unless I apply the shlib patch.  I don't know how to advance the subject.  I tried exactly the same FLAGS variables as jwe reported in comment #17.  The segfault is still there, although the backtrace is slightly different.

I think there must be something related to shared libraries because jwe's segfault in qr.cc is also in a DLDFCN.

#0  0x00007ffff7de30b4 in do_lookup_x (
    undef_name=undef_name@entry=0x7ffff1e5e275 "_ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_PN12octave_shlib9shlib_repEESt10_Select1stISB_ESt4lessIS5_ESaISB_EE5clearEv", new_hash=new_hash@entry=4241444324, old_hash=old_hash@entry=0x7fffd349ca10, ref=0x7ffff1c725e8,
    result=result@entry=0x7fffd349ca20, scope=<optimized out>, i=0, version=0x0, flags=5, skip=0x0, type_class=1, undef_map=0x7ffff7fc5a78)
    at dl-lookup.c:366
#1  0x00007ffff7de3c51 in _dl_lookup_symbol_x (
    undef_name=0x7ffff1e5e275 "_ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_PN12octave_shlib9shlib_repEESt10_Select1stISB_ESt4lessIS5_ESaISB_EE5clearEv", undef_map=0x7ffff7fc5a78, ref=ref@entry=0x7fffd349cad8, symbol_scope=0x7ffff7fc5dd0, version=0x0,
    type_class=type_class@entry=1, flags=5, skip_map=0x0) at dl-lookup.c:775
#2  0x00007ffff7de89ed in _dl_fixup (l=<optimized out>, reloc_arg=<optimized out>) at ../elf/dl-runtime.c:111
#3  0x00007ffff7deff70 in _dl_runtime_resolve () at ../sysdeps/x86_64/dl-trampoline.S:87
#4  0x00007ffff32ce2b9 in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> > >::_M_erase_aux (this=0x7ffff3f722e0 <octave_shlib::shlib_rep::instances[abi:cxx11]>, __first=..., __last=...) at /usr/include/c++/5/bits/stl_tree.h:2260
#5  0x00007ffff32cc62f in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> > >::erase[abi:cxx11](std::_Rb_tree_const_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> >, std::_Rb_tree_const_iterator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> >) (this=0x7ffff3f722e0 <octave_shlib::shlib_rep::instances[abi:cxx11]>, __first=..., __last=...)
    at /usr/include/c++/5/bits/stl_tree.h:1057
#6  0x00007ffff32cb22a in std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*>, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> > >::erase (this=0x7ffff3f722e0 <octave_shlib::shlib_rep::instances[abi:cxx11]>, __x=...) at /usr/include/c++/5/bits/stl_tree.h:2274
#7  0x00007ffff32ca69d in std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, octave_shlib::shlib_rep*, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, octave_shlib::shlib_rep*> > >::erase (this=0x7ffff3f722e0 <octave_shlib::shlib_rep::instances[abi:cxx11]>, __x=...)
    at /usr/include/c++/5/bits/stl_map.h:745
#8  0x00007ffff32ca531 in octave_shlib::shlib_rep::~shlib_rep (this=0x60c0000aabc0, __in_chrg=<optimized out>) at liboctave/util/oct-shlib.h:59
#9  0x00007ffff32c9e89 in octave_dlopen_shlib::~octave_dlopen_shlib (this=0x60c0000aabc0, __in_chrg=<optimized out>) at liboctave/util/oct-shlib.cc:219
#10 0x00007ffff32c9eb8 in octave_dlopen_shlib::~octave_dlopen_shlib (this=0x60c0000aabc0, __in_chrg=<optimized out>) at liboctave/util/oct-shlib.cc:223
#11 0x00007ffff554f66b in octave_shlib::operator= (this=0x6110000f8aa0, sl=...) at ./liboctave/util/oct-shlib.h:137
#12 0x00007ffff555e192 in octave_shlib::close (this=0x6110000f8aa0, cl_hook=0x0) at ./liboctave/util/oct-shlib.h:159
#13 0x00007ffff555c97e in octave_shlib_list::do_remove (this=0x60300023cf80, shl=..., cl_hook=0x0) at libinterp/corefcn/dynamic-ld.cc:116
#14 0x00007ffff555cf96 in octave_shlib_list::remove (shl=..., cl_hook=0x0) at libinterp/corefcn/dynamic-ld.cc:179
#15 0x00007ffff555dae2 in octave_dynamic_loader::do_remove_oct (this=0x60200011f710, fcn_name=..., shl=...) at libinterp/corefcn/dynamic-ld.cc:370
---Type <return> to continue, or q <return> to quit---
#16 0x00007ffff555dc16 in octave_dynamic_loader::remove_oct (fcn_name=..., shl=...) at libinterp/corefcn/dynamic-ld.cc:418
#17 0x00007ffff51fb05f in octave_dld_function::~octave_dld_function (this=0x6110000f89c0, __in_chrg=<optimized out>)
    at libinterp/octave-value/ov-dld-fcn.cc:58
#18 0x00007ffff51fb0be in octave_dld_function::~octave_dld_function (this=0x6110000f89c0, __in_chrg=<optimized out>)
    at libinterp/octave-value/ov-dld-fcn.cc:59
#19 0x00007ffff6983899 in octave_value::operator= (this=0x61400012c180, a=...) at ./libinterp/octave-value/ov.h:364
#20 0x00007ffff539f55b in symbol_table::fcn_info::fcn_info_rep::clear_autoload_function (this=0x61400012c040, force=true)
    at libinterp/corefcn/symtab.h:873
#21 0x00007ffff59d76d3 in symbol_table::fcn_info::fcn_info_rep::clear (this=0x61400012c040, force=true) at libinterp/corefcn/symtab.h:907
#22 0x00007ffff59d79e5 in symbol_table::fcn_info::clear (this=0x6070006b81c0, force=true) at libinterp/corefcn/symtab.h:1110
#23 0x00007ffff59d7eae in symbol_table::clear_functions (force=true) at libinterp/corefcn/symtab.h:1704
#24 0x00007ffff59d7d09 in symbol_table::clear_all (force=true) at libinterp/corefcn/symtab.h:1676
#25 0x00007ffff59d4ead in symbol_table::cleanup () at libinterp/corefcn/symtab.cc:1622
#26 0x00007ffff5a0a436 in do_octave_atexit () at libinterp/corefcn/toplev.cc:853
#27 0x00007ffff5a0c299 in clean_up_and_exit (status=0, safe_to_return=true) at libinterp/corefcn/toplev.cc:890
#28 0x00007ffff4baa1af in octave_execute_interpreter () at libinterp/octave.cc:999
#29 0x00007ffff69d827d in octave_interpreter::execute (this=0x60300004aa70) at libgui/src/octave-interpreter.cc:48
#30 0x00007ffff6a7460b in octave_interpreter::qt_static_metacall (_o=0x60300004aa70, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x6020000251d0)
    at libgui/src/moc-octave-interpreter.cc:57
#31 0x00007fffef75dd01 in QObject::event(QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#32 0x00007fffefc6fcdc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#33 0x00007fffefc76c16 in QApplication::notify(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtGui.so.4
#34 0x00007fffef74385d in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#35 0x00007fffef747316 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#36 0x00007fffef77407e in ?? () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#37 0x00007fffe9af5ff7 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#38 0x00007fffe9af6250 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#39 0x00007fffe9af62fc in g_main_context_iteration () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#40 0x00007fffef7741ee in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#41 0x00007fffef7420d1 in QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#42 0x00007fffef742445 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#43 0x00007fffef6314f9 in QThread::exec() () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#44 0x00007fffef633d1c in ?? () from /usr/lib/x86_64-linux-gnu/libQtCore.so.4
#45 0x00007ffff12dc6aa in start_thread (arg=0x7fffd349e700) at pthread_create.c:333
#46 0x00007ffff1011e9d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Rik <rik5>
Project Administrator
Tue 15 Mar 2016 08:31:36 PM UTC, comment #18: 

When I just run make test (instead of running tests in batches) I see the following result:

  libinterp/dldfcn/fftw.cc-tst ................................ PASS     13/13
  libinterp/dldfcn/qr.cc-tst ..................................ASAN:SIGSEGV
=================================================================
==13081==ERROR: AddressSanitizer: stack-overflow on address 0x2ab5406a2448 (pc 0x2ab51de69e63 bp 0x2ab540726470 sp 0x2ab5406a2380 T6)
    #0 0x2ab51de69e62 in zgetrf_parallel (/usr/lib/libopenblas.so.0+0x319e62)
    #1 0x2ab51de6a017 in zgetrf_parallel (/usr/lib/libopenblas.so.0+0x31a017)
    #2 0x2ab51de6a017 in zgetrf_parallel (/usr/lib/libopenblas.so.0+0x31a017)
    #3 0x2ab51de6a017 in zgetrf_parallel (/usr/lib/libopenblas.so.0+0x31a017)
    #4 0x2ab518f68f46 in zgetrf_ (/usr/lib/liblapack.so.3+0x25f46)
    #5 0x2ab512bdf199 in ComplexMatrix::fsolve(MatrixType&, ComplexMatrix const&, int&, double&, void (*)(double), bool) const /home/jwe/src/octave/liboctave/array/CMatrix.cc:2147
    #6 0x2ab512be0967 in ComplexMatrix::solve(MatrixType&, ComplexMatrix const&, int&, double&, void (*)(double), bool, blas_trans_type) const /home/jwe/src/octave/liboctave/array/CMatrix.cc:2291
    #7 0x2ab512be0249 in ComplexMatrix::solve(MatrixType&, Matrix const&, int&, double&, void (*)(double), bool, blas_trans_type) const /home/jwe/src/octave/liboctave/array/CMatrix.cc:2241
    #8 0x2ab510feba7c in xleftdiv(ComplexMatrix const&, Matrix const&, MatrixType&, blas_trans_type) /home/jwe/src/octave/libinterp/corefcn/xdiv.cc:387
    #9 0x2ab51027ab35 in oct_binop_ldiv /home/jwe/src/octave/libinterp/operators/op-cm-m.cc:84

John W. Eaton <jwe>
Project Administrator
Tue 15 Mar 2016 08:26:39 PM UTC, comment #17: 

Rik:

Yes, I looked at the classdef leak.  I see where it is coming from.  The classdef object created in the parser is never cleaned up, but I'm not sure yet where or when that should happen.

I don't see a crash with "test imread".

I'm building on an x86_64 Debian testing system.  I used

.../configure --disable-java --enable-address-sanitizer-flags --disable-jit --without-opengl --disable-docs --prefix=/usr/local/octave/dev-asan --with-hdf5-includedir=/usr/include/hdf5/serial --with-hdf5-libdir=/usr/lib/x86_64-linux-gnu/hdf5/serial CFLAGS="-O0 -ggdb3" CXXFLAGS="-O0 -ggdb3" FFLAGS="-O0 -ggdb3"

I'm running a full make check now to see if I have any crashes with this configuration.  When I ran the tests in batches, I did see leaks for anything that used classdef, but I didnt't see any crashes.

I do see that my copy of Octave built with these options won't start the GUI.  It just exits without any message.  So all my tests have been done with "octave --no-gui"

There are some Qt and fontconfig leaks as well.  I'm guessing those are happening because we aren't really deleting things when shutting down the GUI.  It would be nice to fix those as well, but I'm not too concerned about them at the moment.

John W. Eaton <jwe>
Project Administrator
Tue 15 Mar 2016 07:51:49 PM UTC, comment #16: 

I have tested the patch from comment #12 under Windows, and

  • it works wonderfully to eliminate the long standing crashing that seem to come from memory leaks.
  • But unfortunately this patch also revives another (already closed) bug that also creates crashes.

Here are the details:

I added the mentioned small patch as "stable-octave-1-testimagesegfaults.patch" to the src folder of the MXE Octave source directory. Then I compiled MXE Octave with the current tip of the Octave stable branch:

autoconf
./configure
make zip-dist

I unpacked the zip file on a Windows 7 machine and started bin/octave-gui.exe.

Here are the results of my tests:
A) When I run this

test imread
clear all
test imread

Then

  • with Octave-4.0.1-rc4: I get an immediate crash of the Octave GUI
  • with the patched version: no crash

B) When I run this

test imread
exit

Then

  • with Octave-4.0.1-rc4: I get a segfault on exit
  • with the patched version: no segfault

C) I run several image processing scripts. I could NOT produce any segfaults on exit.

D) While running my personal set of image processing scripts, I sadly discovered that bug #45372 came back to life with this patch!
Here is an example of how to produce the crash of this (already closed) bug report:

I = uint8(100.*ones(20,20,3));
imshow(I)
% now click with the left mouse button into the figure window

Observation:

  • with Octave-4.0.1-rc4: no problem (bug has already been fixed)
  • with the patched version: immediate crash of the GUI when clicking

My personal conclusion regarding this patch: I consider the "crash on click" bug even more severe, compared to the two (memory leak induced) bugs "clear all provokes crashes" and "segfault on exit". So unless the revived bug #45372 can be easily wiped out for this patch, I would (very sadly) NOT vote to include this patch into a new Octave-4.0.1-rc5 release candidate.

Hartmut <hardy>
Tue 15 Mar 2016 07:14:13 PM UTC, comment #15: 

@jwe: The classdef leak is from inputParser.m.  The image directory has segfaults which prevent further testing unless my pach was applied.

Rik <rik5>
Project Administrator
Tue 15 Mar 2016 03:48:29 AM UTC, comment #14: 

Rik:

I also disabled Java to avoid having to think about that for now.

I'm seeing the problems with classdef code, but not for the image directory, even without your patch.  Maybe the problem is with the version of GraphicsMagick that you have?  What version are you using?  I have the x86_64  Debian packages with the version number 1.3.23-1+b1.

John W. Eaton <jwe>
Project Administrator
Tue 15 Mar 2016 03:43:28 AM UTC, comment #13: 

I checked in two changesets for potential leaks that I found with the address sanitizer.

http://hg.savannah.gnu.org/hgweb/octave/rev/ac1e50cfc9ed
http://hg.savannah.gnu.org/hgweb/octave/rev/06a2f9245a11

I'm not sure exactly which tests uncovered these for me as I was running them in batches (I modified _run_test_suite_.m to allow me to run a subset of the tests).

John W. Eaton <jwe>
Project Administrator
Sun 13 Mar 2016 07:20:50 PM UTC, comment #12: 

I'm attaching a quick patch which seems to resolve all of the segfaults as well as the memory leaks in the image directory.  This is probably not the way to do it--I just did it as a quick test--but if it resolves the numerous strage crashes on Windows platforms then this might be worth stuffing in to a 4.0.1-rc5 candidate.

diff -r de155ca994d4 liboctave/util/oct-shlib.h
--- a/liboctave/util/oct-shlib.h        Sun Mar 13 08:46:59 2016 -0700
+++ b/liboctave/util/oct-shlib.h        Sun Mar 13 12:17:10 2016 -0700
@@ -119,8 +119,8 @@ public:

   ~octave_shlib (void)
   {
-    if (--rep->count == 0)
-      delete rep;
+    //if (--rep->count == 0)
+     // delete rep;
   }

   octave_shlib (const octave_shlib& sl)

(file #36622)

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 07:00:00 PM UTC, comment #11: 

The segfaults I believe are all related to Octave and the way it is using the ImageMagick library.  There is much more information at bug #41699 about this same behavior on a Windows platform.  I was able to get a backtrace on Linux shown below.  It is definitely during the shutdown process when the shared library is being unloaded.

#0  0x00007ffff52da425 in octave_refcount<int>::operator++ (this=0x60c00006f100)
    at ./liboctave/util/oct-refcount.h:66
#1  0x00007ffff5982cd5 in octave_shlib::operator= (this=0x6110000da1e0, sl=...)
    at ./liboctave/util/oct-shlib.h:140
#2  0x00007ffff5982de4 in octave_shlib::close (this=0x6110000da1e0, cl_hook=0x0)
    at ./liboctave/util/oct-shlib.h:159
#3  0x00007ffff5c9cd12 in octave_shlib_list::do_remove (this=0x6030001f5070,
    shl=..., cl_hook=0x0) at libinterp/corefcn/dynamic-ld.cc:116
#4  0x00007ffff5c9d32a in octave_shlib_list::remove (shl=..., cl_hook=0x0)
    at libinterp/corefcn/dynamic-ld.cc:179
#5  0x00007ffff5c9de76 in octave_dynamic_loader::do_remove_oct (this=0x6020001173d0,
    fcn_name=..., shl=...) at libinterp/corefcn/dynamic-ld.cc:370
#6  0x00007ffff5c9dfaa in octave_dynamic_loader::remove_oct (fcn_name=..., shl=...)
    at libinterp/corefcn/dynamic-ld.cc:418
#7  0x00007ffff59235dd in octave_dld_function::~octave_dld_function (
    this=0x6110000da100, __in_chrg=<optimized out>)
    at libinterp/octave-value/ov-dld-fcn.cc:58
#8  0x00007ffff592363c in octave_dld_function::~octave_dld_function (
    this=0x6110000da100, __in_chrg=<optimized out>)
    at libinterp/octave-value/ov-dld-fcn.cc:59
#9  0x00007ffff52d5119 in octave_value::operator= (this=0x61400011cf80, a=...)
    at ./libinterp/octave-value/ov.h:364
#10 0x00007ffff5ae16b1 in symbol_table::fcn_info::fcn_info_rep::clear_autoload_function (this=0x61400011ce40, force=true) at libinterp/corefcn/symtab.h:873
#11 0x00007ffff6117765 in symbol_table::fcn_info::fcn_info_rep::clear (
    this=0x61400011ce40, force=true) at libinterp/corefcn/symtab.h:907
---Type <return> to continue, or q <return> to quit---
#12 0x00007ffff6117a77 in symbol_table::fcn_info::clear (this=0x6070006975a0,
    force=true) at libinterp/corefcn/symtab.h:1110
#13 0x00007ffff6117f40 in symbol_table::clear_functions (force=true)
    at libinterp/corefcn/symtab.h:1704
#14 0x00007ffff6117d9b in symbol_table::clear_all (force=true)
    at libinterp/corefcn/symtab.h:1676
#15 0x00007ffff6114f3f in symbol_table::cleanup ()
    at libinterp/corefcn/symtab.cc:1622
#16 0x00007ffff6132260 in do_octave_atexit () at libinterp/corefcn/toplev.cc:853
#17 0x00007ffff61340c3 in clean_up_and_exit (status=0, safe_to_return=true)
    at libinterp/corefcn/toplev.cc:890
#18 0x00007ffff52d2a0f in octave_execute_interpreter () at libinterp/octave.cc:999
#19 0x0000000000402c6b in main (argc=9, argv=0x7fffffffca38) at src/main-cli.cc:42

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 06:30:51 PM UTC, comment #10: 

Summary of open issues as of 3/13/16:

leaks in general/inputParser.m from classdef code
leaks in java/ directory due to JVM
leak in image/imfinfo.m (8 bytes)
leak in image/imformats.m (8 bytes)
segfault in image/imread.m
segfault in image/iwrite.m
segfault in plot/util/copyobj.m
segfault in plot/util/hgsave.m

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 05:43:42 PM UTC, comment #9: 

The leaks in the general/ directory are related to fieldnames.m, methods.m, and inputParser.m.  The first two files are actually leaking because they invoke the Java JVM.  The inputParser, however, is a different story and there are leaks in the classdef code.  This is not my area of expertise, so I have attached the leak log for this file for someone else to review. 

(file #36621)

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 03:50:10 PM UTC, comment #8: 

The memory leak in the optimization directory is because Octave was not calling glp_free_env() to ask GLPK to completely free resources used.  Fixed in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/de155ca994d4).

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 02:43:03 PM UTC, comment #7: 

The memory leak in miscellaneous/ is also due to the use of putenv in a BIST test.  This doesn't require fixing either.

Rik <rik5>
Project Administrator
Sun 13 Mar 2016 02:37:47 PM UTC, comment #6: 

The memory leak in the prefs directory is due to the putenv call.  This is a known issue and apparently almost unavoidable when using either putenv or setenv calls.  I added a note about this leak in lo-utils.cc (http://hg.savannah.gnu.org/hgweb/octave/rev/74b07936fd05).

Rik <rik5>
Project Administrator
Sat 12 Mar 2016 01:26:09 AM UTC, comment #5: 

I built Octave as described in the original bug report.  I then tried to search for other memory leaks in Octave by running

./run-octave -f --no-gui --eval '__run_test_suite__; exit'

This does make it through the test suite, but then segfaults on exit without producing a report on the leaks.

I changed tactics and started octave as before, but then ran 'runtests scripts/XXX' followed by exit.  Most directories showed no leaks, but the following had problems.

general/  : Extra leaks
image/    : Segfault on exit
java/     : Extra leaks
miscellaneous/ : Small extra leak
optimization/ : Extra leaks
plot/util : Segfault on exit
prefs     : Small extra leak (probably due to setenv)

I think this tool may help us get to the bottom of some longstanding strange behavior we have seen.  For example, running a java command is enough to change the output of svds.  That could be explained by the massive memory leaks that are happening when the JVM is used.  Similarly, on Windows there has been a longstanding bug that whenever imformats was used, Octave would segfault on exit.  I think that probably explains the segfault in the image/ directory.

Rik <rik5>
Project Administrator
Sat 12 Mar 2016 01:17:15 AM UTC, comment #4: 

@jwe: I've verified that all of the memory leaks originally mentioned having to do with octave-link have been plugged.

I'm finding the AddressSanitizer to be a quite useful tool.  It is far faster than valgrind which allows one quickly run tests.  I'm going to re-title and re-purpose this bug report to general memory leaks in Octave.

Rik <rik5>
Project Administrator
Fri 11 Mar 2016 09:35:49 PM UTC, comment #3: 

I checked in the following changeset to fix the leaks that are coming directly from Octave constructor/destructor mismatch:

http://hg.savannah.gnu.org/hgweb/octave/rev/062c65569ad7

After this, I still can't start the GUI when I've built with the address sanitizer option.  It seems that the GUI subprocess is not starting properly but I haven't investigated.

Starting with the --no-gui option does seem to work.  On exit, there are a number of messages about leaks but those all appear to be coming from libraries that Octave uses, not directly from Octave.

John W. Eaton <jwe>
Project Administrator
Fri 11 Mar 2016 06:48:15 PM UTC, comment #2: 

@Mike: There was no change in the amount of memory leaked after your patch to address the race condition at GUI startup.  I'm not that surprised, the leaks reported seem to be classic failures to pair new/delete rather than cross-thread issues.

Rik <rik5>
Project Administrator
Thu 10 Mar 2016 08:24:38 PM UTC, comment #1: 
Mike Miller <mtmiller>
Project Administrator
Wed 09 Mar 2016 06:34:37 PM UTC, original submission:  

gcc has a fast, lightweight memory checker that can be enabled with compiler flags.  To check Octave, I did the following (this is for a csh shell).

setenv CFLAGS "-g -O0 -pipe"
setenv CXXFLAGS "${CFLAGS}"
setenv FFLAGS "${CFLAGS}"
./configure --enable-address-sanitizer-flags --disable-jit --without-opengl --disable-docs
make

There are numerous problems in the OpenGl libraries so they had to be disabled.  Likewise, the build won't complete without --disable-docs because the build system uses run-octave to generate images and that requires OpenGl libraries at the moment.

With the instrumented version of Octave in place, I ran

./run-octave -f --no-gui-libs
exit

This produces a clean report.  There are no memory leaks when using the CLI binary which is not linked against Qt.  Next, I tried

./run-octave -f --no-gui
exit

This produces a list of leaks which I have attached as leak.log.  In general, the leaks appear valid.  The constructors for octave-link, octave-qt-link, octave-mutex use the new operator to initialize some member variables.  But it appears that the destructors are empty functions (some of them virtual as if it was expected that they might be overriden) which never free this allocated memory.  It should be relatively easy to fix these.

Finally, I can't run the GUI at all because there is an immediate memory leak in the octave binary which is designed to switch between running octave-gui or octave-cli.

./run-octave -f

=================================================================
==20003==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x7f75e3c94a32 in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99a32)
    #1 0x4055c4 in main src/main.cc:449
    #2 0x7f75e2a58a3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20a3f)

Indirect leak of 54 byte(s) in 1 object(s) allocated from:
    #0 0x7f75e3c94a32 in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99a32)
    #1 0x40539b in strsave src/main.cc:420
    #2 0x405a93 in main src/main.cc:520
    #3 0x7f75e2a58a3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20a3f)

SUMMARY: AddressSanitizer: 134 byte(s) leaked in 2 allocation(s).

I was working from revision 21419:13415264b9f8 so all of the line numbers are accurate for that tip.

Rik <rik5>
Project Administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36868:  java-mem.cc added by jwe (1KiB - text/x-c++src)
file #36866:  java-leak-log.txt added by jwe (225KiB - text/plain)
file #36845:  leak-log.txt added by jwe (31KiB - text/plain)
file #36787:  diffs.txt added by jwe (635B - text/plain)
file #36786:  shlib_magick.cset added by rik5 (1KiB - application/octet-stream)
file #36754:  patch-1.txt added by jwe (914B - text/plain)
file #36755:  patch-2.txt added by jwe (1017B - text/plain)
file #36622:  octshlib.patch added by rik5 (381B - application/x-download)
file #36621:  inputParser.leak.log added by rik5 (721KiB - text/x-log)
file #36583:  leak.log added by rik5 (9KiB - text/x-log)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by avinoam (Posted a comment)
  • -email is unavailable- added by avinoam
  • -email is unavailable- added by hardy (Posted a comment)
  • -email is unavailable- added by hardy
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (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 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-05-13 rik5 StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2016-04-12 jwe Attached File- => Added leak-log-for-demos-with-opengl.txt, #36910
    2016-04-08 rik5 StatusFixed => In Progress
        Open/ClosedClosed => Open
    2016-04-08 rik5 StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2016-04-08 jwe Attached File- => Added java-mem.cc, #36868
    2016-04-08 jwe Attached File- => Added java-leak-log.txt, #36866
    2016-04-06 jwe Attached File- => Added leak-log.txt, #36845
    2016-03-29 jwe Attached File- => Added diffs.txt, #36787
    2016-03-28 rik5 Attached File- => Added shlib_magick.cset, #36786
    2016-03-24 jwe Attached File- => Added patch-1.txt, #36754
        Attached File- => Added patch-2.txt, #36755
    2016-03-24 rik5 Carbon-Copy- => Added carandraug
    2016-03-18 avinoam Carbon-Copy- => Added avinoam
    2016-03-14 hardy Carbon-Copy- => Added hardy
    2016-03-13 rik5 Attached File- => Added octshlib.patch, #36622
        StatusConfirmed => In Progress
    2016-03-13 rik5 Item GroupPerformance => Segfault, Bus Error, etc.
    2016-03-13 rik5 Dependencies- => Depends on bugs #41699
    2016-03-13 rik5 Attached File- => Added inputParser.leak.log, #36621
    2016-03-13 rik5 StatusNone => Confirmed
    2016-03-12 rik5 Summarymemory leaks in octave-link, octave-qt-link, octave-mutex => Memory leaks and segmentation faults in Octave
    2016-03-09 rik5 Attached File- => Added leak.log, #36583

    Back to the top


    Powered by Savane 3.5