Re: garbage collection (was Re: gpfree_string within temp_at())

[Ethan M. <merritt@u.washington.edu>]

On Friday 08 July 2005 01:57 am, Dave Denholm wrote:
> That reminds me...  there's a couple of memory/cleanup issues been
> mentioned recently, and IIRC, there used to be a problem with
> memory leaks if SIGINT is sent during a plot, since the exact state is
> unknown, and so data structures cannot safely be traversed and freed.

SIGINT while actually in the eval_plots code must be a rare occurance,
so I'm not sure how much effort we should put into worrying about it.
SIGINT to terminate a "pause for mouse" is much more common, but I can't
see why that would lead to any leaks.
 
> allocated blocks are kept in linked lists : say a list of "permanent"
> blocks (eg udf's)

This is already done. The listhead for udvs is first_udv, the listhead
for labels is first_label, and so on. 

> and a list of transient blocks (allocated during a 
> plot, for example). 

Hmmm.  I think this is also already done, although maybe I am overlooking
a category of allocated space.  The major dynamic allocations for a plot
are referenced by elements of the corresponding   struct curve_points,
and these structures are themselves linked into a list.

> At end of a plot, everything in the transient list is reclaimed.

The previous allocations are freed at the time a new plot is built up.
This should work even if the previous plot was interrupted by SIGINT.
 
> Need to be a little careful with registration of non-trivial objects,
> to make sure things are atomic wrt SIGINT. 

That is indeed an issue in principle (but hard to debug I think!).

New allocations (udv, whole plots, ...) are added at the end of a
linked list. But this allocate+link operation is not guaranteed 
atomic by the current code.  I'm not particularly worried by the
possibily of leaking a single newly-allocated block. But if the
SIGINT were to occur at just the  wrong time, the final list link
pointer itself could become corrupt. That would show up as a
segfault rather than a memory leak.

   new_element = &(current_end_of_list->next);
   new_element = gp_alloc(...);
                    <<<<  SIGINT here is bad news
   new_element->next = NULL;

This is fixable, either by careful audit of all the routines that
do list insertion or by modifying gp_alloc to zero out new space
before returning it.  

Am I missing other issues?


-- 
Ethan A Merritt       merritt@u.washington.edu
Biomolecular Structure Center
Mailstop 357742
University of Washington, Seattle, WA 98195