Re: segfault and onCleanup()

[Ben Abbott]

On Dec 8, 2011, at 10:12 AM, Ben Abbott wrote:

> On Dec 8, 2011, at 6:59 AM, John W. Eaton wrote:
> 
>> On  7-Dec-2011, Rik wrote:
>> 
>> | On 12/07/2011 12:41 PM, John W. Eaton wrote:
>> | > I'm not seeing this crash, but I checked in this change as a temporary
>> | > fix for those of you who are.
>> | >
>> | > I don't know that I'll be able to debug this problem unless I can
>> | > find a way to reproduce it myself.
>> | 
>> | I've single-stepped through the segfault.  The trouble is not directly in
>> | cleanup_instance().  Rather the code 'delete instance;' triggers the 
>> destructor
>> | for the class.  The destructor defined in ov-typeinfo.h is empty, but 
>> there are
>> | a number of Array objects in the class and the destructor for the Array 
>> class
>> | is called.  It is not the first element of the Array object which triggers 
>> the
>> | segfault.  It seems to be the 14th but I don't know if that is the 14th 
>> element
>> | of the Array or the 14th Array object.  It also might simply be a red 
>> herring. 
>> | The relevant part of the trace is below.
>> | 
>> | #18 0x00002aaaad757460 in _int_free (av=0x7fffffffd1f0, p=0x0) at 
>> malloc.c:5017
>> | #19 0x00002aaaab1059cb in ~octave_value (this=0x63ce70, __in_chrg=<value
>> | optimized out>) at ov.h:311
>> | #20 0x00002aaaab106c8f in ~ArrayRep (this=0x63ce70, __in_chrg=<value 
>> optimized
>> | out>) at ../liboctave/Array.h:94
>> | #21 0x00002aaaab106321 in ~Array (this=0x62cfe8, __in_chrg=<value optimized
>> | out>) at ../liboctave/Array.h:235
>> | #22 0x00002aaaab5b3582 in ~octave_value_typeinfo (this=0x62cfc0, __in_chrg=
>> | <value optimized out>) at ov-typeinfo.h:192
>> | #23 0x00002aaaab5b3f67 in octave_value_typeinfo::cleanup_instance () at
>> | ov-typeinfo.h:223
>> | #24 0x00002aaaac3ea2c8 in ~singleton_cleanup_list (this=0x612380, 
>> __in_chrg=
>> | <value optimized out>) at singleton-cleanup.cc:39
>> | #25 0x00002aaaab4c02a1 in singleton_cleanup_list::cleanup () at 
>> ../liboctave/
>> | singleton-cleanup.h:26
>> | #26 0x00002aaaab4bd280 in clean_up_and_exit (retval=0) at toplev.cc:697
>> | #27 0x00002aaaab46f5d1 in octave_main (argc=11, argv=0x7fffffffd1f8, 
>> embedded=
>> | 0) at octave.cc:904
>> | #28 0x0000000000400769 in main (argc=11, argv=0x7fffffffd1f8) at main.c:35
>> | 
>> | Also, most of the time I can start 'run-octave' and mess around in the
>> | interpreter without problems.  However, it is always reproducible with 
>> 'make
>> | check'.  I did some verification with runtests and the following functions
>> | always cause a segfault when exiting octave if I have used 'test XXX'.
>> | 
>> | ./linear-algebra/onenormest.m
>> | ./geometry/griddata3.m
>> | ./geometry/delaunay.m
>> | ./plot/trisurf.m
>> | ./plot/triplot.m
>> | ./plot/trimesh.m
>> | ./sparse/svds.m
>> | ../src/DLD-FUNCTIONS/onCleanup.cc
>> | 
>> | The common element (clue?) is that they all use onCleanup within a %!test
>> | block.  If I comment out the onCleanup () call then the segfault 
>> disappears. 
>> | Actually, on further trolling through the code, these are the only points 
>> where
>> | onCleanup is ever used.  I did a quick test with the following in the file
>> | blah.m
>> | 
>> | function blah ()
>> |   restore_state = onCleanup (@() disp ("hello"));
>> |   disp ("First");
>> | endfunction
>> | 
>> | Calling blah() correctly displays
>> | First
>> | hello
>> | 
>> | but this segfaults as well when I exit Octave.  This problem isn't 
>> confined to
>> | some weirdness of %!test blocks.
>> | 
>> | I'm of the opinion that the whole onCleanup class is broken.  Perhaps you 
>> can
>> | see quickly what is wrong.  Otherwise, the %!test blocks can be rewritten 
>> to
>> | use unwind_protect blocks to accomplish the same task of restoring the 
>> random
>> | seed.
>> 
>> I don't see anything really wrong with onCleanup except that it is
>> dynamically loaded.  So I checked in a change to build it into
>> liboctinterp so that clearing it should not be able to cause trouble.
>> 
>> I also checked in some changes for gh_manager and reactivated all the
>> singleton cleanup_instance functions.  As part of the gh_manager
>> changes, errors in closerequestfcn callback functions should be
>> handled better than I think they were before.
>> 
>> You'll need to be sure to remove the obsolete onCleanup.oct file
>> from your build directory.
>> 
>> I'm not seeing any crashes now.  I've run make check multiple times and
>> also ran through all the plot demos without seeing Octave crashing
>> when it exits.
>> 
>> If you see failures, please let me know what OS (32 or 64 bit?),
>> compiler version, etc. that you are using so I can try to duplicate
>> the problems you are seeing.
>> 
>> jwe
> 
> John, I'm getting an error due to missing ov-cleanup.h. Should the #include 
> be ...
> 
> diff --git a/src/ov-oncleanup.cc b/src/ov-oncleanup.cc
> --- a/src/ov-oncleanup.cc
> +++ b/src/ov-oncleanup.cc
> @@ -25,7 +25,7 @@
> #endif
> 
> #include "defun.h"
> -#include "ov-cleanup.h"
> +#include "ov-oncleanup.h"
> #include "ov-fcn.h"
> #include "ov-usr-fcn.h"
> #include "pt-misc.h"
> 
> Ben

There's another in ov.cc

$ hg diff
diff --git a/src/ov-oncleanup.cc b/src/ov-oncleanup.cc
--- a/src/ov-oncleanup.cc
+++ b/src/ov-oncleanup.cc
@@ -25,7 +25,7 @@
 #endif
 
 #include "defun.h"
-#include "ov-cleanup.h"
+#include "ov-oncleanup.h"
 #include "ov-fcn.h"
 #include "ov-usr-fcn.h"
 #include "pt-misc.h"
diff --git a/src/ov.cc b/src/ov.cc
--- a/src/ov.cc
+++ b/src/ov.cc
@@ -65,7 +65,7 @@
 #include "ov-range.h"
 #include "ov-struct.h"
 #include "ov-class.h"
-#include "ov-cleanup.h"
+#include "ov-oncleanup.h"
 #include "ov-cs-list.h"
 #include "ov-colon.h"
 #include "ov-builtin.h"

Ben