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.
|
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.
|
Fri 29 Apr 2016 03:34:25 PM UTC, comment #119:
Rik, RE the crash for
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
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.
|
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:
http://doc.qt.io/qt-4.8/objecttrees.html
http://doc.qt.io/qt-5/qstringlistmodel.html
|
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
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.
|
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
|
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
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:
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)
|
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.
|
Fri 08 Apr 2016 09:49:24 PM UTC, comment #113:
Fun!
|
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.
With that done, I can now find new, ugly memory violations.
For instance,
which always causes a heap-buffer-overflow error for me.
|
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.
|
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)
|
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.
Second, does the stanza
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.
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.
|
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
Since _java_exit_ is supposed to terminate the jvm and unload the jvm shared library I would expect no leaks.
(file #36866)
|
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.
|
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:
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.
|
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.
|
Thu 07 Apr 2016 04:11:24 AM UTC, comment #104:
@John, could you upload your installer to http://alpha.gnu.org/gnu/octave ?
|
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?
|
Thu 07 Apr 2016 03:43:18 AM UTC, comment #102:
RE: comment #86, I can run
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?
|
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.
|
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
This might be because I don't have debug-enabled libraries for Qt.
|
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.
|
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
and
And I see
for the case without _java_exit_ and
for the case with it.
WTF?
|
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.
|
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. :-/
|
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
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?
|
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.
|
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.
|
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_.
|
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):
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.
|
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
I have been testing by executing this statement from the command line
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?
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?
|
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)
|
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.
|
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.
|
Wed 06 Apr 2016 04:12:05 AM UTC, comment #86:
Building MXE-Octave from scratch (changeset 4c61f8313f58, still
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?
|
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.
|
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.
|
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).
|
Mon 04 Apr 2016 04:17:36 AM UTC, comment #82:
sorry: should be
@Hartmut (comment #79)
|
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.
|
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)
|
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).
|
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.
|
Sun 03 Apr 2016 04:49:24 AM UTC, comment #77:
Addition to comment #76:
Only the GUI version segfaults. The CLI does not.
|
Sun 03 Apr 2016 04:18:09 AM UTC, comment #76:
checking the sequence
Under Windows (Win-7), it still segfaults.
|
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.
|
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.
|
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.
|
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.
|
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:
|
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.
|
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.
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.
|
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.
|
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.
|
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.
|
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.)
|
Wed 30 Mar 2016 05:13:03 PM UTC, comment #64:
More interesting links:
http://stackoverflow.com/questions/2479784/exporting-static-data-in-a-dll
http://cboard.cprogramming.com/cplusplus-programming/98673-static-variable-vs-dll.html
|
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.
That seems to be what is happening on Linux, and what definitely does not happen on Windows.
|
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
The empty Matrix object ultimately uses this code from Array<T>:
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
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.
|
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.
|
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.
|
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.
|
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.
|
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)?
|
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.
|
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
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.
|
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)
|
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).
|
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:
Neither of these returns anything so it doesn't appear that libgomp is linked in.
|
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
|
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.
|
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.
|
Mon 28 Mar 2016 09:48:52 PM UTC, comment #48:
Maybe the problem we are seeing is related to the one described here:
http://stackoverflow.com/questions/29117441/tricky-error-using-openmp-in-function-loaded-from-dynamic-libraries
|
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.
|
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)
|
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.
|
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
and I get no segfault when I do
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.
|
Mon 28 Mar 2016 06:36:46 PM UTC, comment #43:
Getting closer... There is no segfault if I do
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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.
|
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?
|
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.
|
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?
|
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)
|
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
|
Thu 24 Mar 2016 06:38:45 PM UTC, comment #31:
People
|
Thu 24 Mar 2016 06:38:02 PM UTC, comment #30:
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
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.
|
Thu 24 Mar 2016 05:58:08 PM UTC, comment #29:
Does something like
trigger a crash for you?
|
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.
According to the man page for dlopen
In either case, I'm preventing the library from being unloaded. Still not clear why it is happening though.
|
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.
|
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
|
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.
|
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.
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.
|
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.
|
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?
|
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.
|
Fri 18 Mar 2016 08:05:44 AM UTC, comment #20:
I built with
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.
|
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.
|
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:
|
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
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.
|
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:
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
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
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:
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.
|
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.
|
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.
|
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).
|
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.
(file #36622)
|
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.
|
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
|
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)
|
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).
|
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.
|
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).
|
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
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.
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.
|
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.
|
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.
|
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.
|
Thu 10 Mar 2016 08:24:38 PM UTC, comment #1:
Any changes to the report after http://hg.savannah.gnu.org/hgweb/octave/rev/217e6e97085b ?
|
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).
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
This produces a clean report. There are no memory leaks when using the CLI binary which is not linked against Qt. Next, I tried
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.
I was working from revision 21419:13415264b9f8 so all of the line numbers are accurate for that tip.
|