patchGNU Octave - Patches: patch #9060, Re-enable singleton cleanup...

 
 

patch #9060: Re-enable singleton cleanup (mostly)

Submitter:  Robert Jenssen <morgawr>
Submitted:  Thu 28 Jul 2016 03:26:41 AM UTC
   
 
Category:  Core : other Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 28 Jul 2016 03:47:57 AM UTC, comment #1: 

Thanks for looking at this.  I think it will probably help to understand the sequence of events.  However, I think this must wait until after 4.2 is released.

I've been working toward eliminating singleton objects as much as possible and placing them inside Octave "application" and "interpreter" objects that will manage the resources that are currently global.  So eventually, it should be much clearer what objects create, own, and use these resources.  That should also make it easier to know when they can be deleted.

John W. Eaton <jwe>
Group administrator
Thu 28 Jul 2016 03:26:41 AM UTC, original submission:  

This patch is a step in my quixotic quest to reduce the number of memory leak
warnings from valgrind and address-sanitizer. This patch report relates to:

1. Bug #44420 "Memory leaks in 4.0 RC"

2. https://lists.gnu.org/archive/html/octave-maintainers/2012-01/msg00039.html
"Re: singleton cleanup issues (was: Re: Fwd: octave segfaults during exit"

3. https://lists.gnu.org/archive/html/octave-maintainers/2011-12/msg00045.html
"Re: segfault on 'make check'"



The first patch is cleanup_1.patch. It makes the following changes:
1. Enables the call to singleton_cleanup_list::cleanup() in
libinterp/corefcn/interpreter.cc. This was commented out to fix the
make_int typeinfo segfault.
2. Disables singleton cleanup by typeinfo::cleanup_instance().
3. Disables singleton cleanup by ft_manager::cleanup_instance().



If ft_manager::cleanup_instance() is enabled, then the following
Octave code segfaults: "plot(0);legend('t');quit;", as shown in the attached
file legend_valgrind.log. It seems that there is graphics_object that
has not been freed by gh_manager::cleanup_instance() but is now being freed
by the finaliser for the liboctinterp.so.3.0.0 shared library. Unfortunately,
the FT face memory in the graphics_object has already been freed by
ft_manager::cleanup_instance(). Using this information I ran the gdb session
shown in legend_gdb.log. The session shows:
1. gh_manager is instantiated before ft_manager so ft_manager::cleanup_instance()
is called before gh_manager::cleanup_instance().
2. The backtrace just before the segfault shows that the finaliser is trying
to delete a std::map object called dprop_obj_map that is instantiated at
graphics.cc:1694:
// Used to cache dummy graphics objects from which dynamic properties can be
// cloned.
static std::map<caseless_str, graphics_object> dprop_obj_map;



The second patch is cleanup_2.patch. In addition to cleanup_1.patch it:
1. Enables ft_manager::cleanup_instance()
2. Forces instantiation of ft_manager by inserting a dummy text_renderer
object before gh_manager in register_graphics_toolkit(). Maybe there is a
better way to do this.
3. Makes dprop_obj_map into a singleton class called dprop_obj_manager
that maps a property name into the graphics_handle of a graphics_object.
4. Makes singleton_cleanup_list use a stack instead of a set so that the
cleanup order is known to be the reverse of the order of addition to the
list.

These changes are intended to force the order of instantiantion to be
ft_manager, gh_manager, dprop_obj_manager and the order of cleanup to be
dprop_obj_manager, gh_manager, ft_manager.



Future things to work on:

1. Add a function to unregister typeinfo so that typeinfo::cleanup_instance()
works with examples/code/make_int.cc. Modify that example to separate
the instantiation of type myint and the instantiation of objects of
type myint. Re-enable typeinfo::cleanup_instance().

2. Look at classdef cleanup

3. Fix FIXME in ft_manager::~ft_manager
Change the order of calls to FcInit and FT_Init_FreeType in
ft_manager::ft_manager(void). I have not found an Octave bug report for
the problem with fccache.c:507. I doubt that this is due to a bug in
fontconfig. I suspect this is more likely due to calls to FcFini() from
both ft_manager::~ft_manager and the fltk library.

4. Look at interpreter memory usage during the processing of startup files


Robert Jenssen <morgawr>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #38037:  legend_gdb.log added by morgawr (28KiB - text/x-log - Patches to mostly re-enable singleton-cleanup)
file #38038:  cleanup_2.patch added by morgawr (7KiB - text/x-patch - Patches to mostly re-enable singleton-cleanup)
file #38039:  legend_valgrind.log added by morgawr (28KiB - text/x-log - Patches to mostly re-enable singleton-cleanup)
file #38036:  cleanup_1.patch added by morgawr (3KiB - text/x-patch - Patches to mostly re-enable singleton-cleanup)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-28 morgawr Attached File- Added cleanup_1.patch, #38036
        Attached File- Added legend_gdb.log, #38037
        Attached File- Added cleanup_2.patch, #38038
        Attached File- Added legend_valgrind.log, #38039

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code