gnuplot-beta

Re: gpfree_string within temp_at()

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

On Wednesday 06 July 2005 02:36 pm, Juergen Wieferink wrote:
> But I have placed the "#ifdef" quite funny, did you notice?  

I noticed. But then, we *had* been discussing whether to remove
the conditional coding around string variables. I figured you
were just off to a quick start :-) 

> I don't think there is much reason to hurry, 
> so I'd suggest to wait.

OK.

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

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

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

[Daniel J S. <dan...@ie...>]

Ethan Merritt wrote:
> On Friday 08 July 2005 01:57 am, Dave Denholm wrote:
>>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.  

I think the only true way to fix that sort of thing is if SIGINT is disabled before starting the memory allocation and re-enabled after recording the pointer in the linked list.  Otherwise, the same problem can still occur inside gp_alloc() I would think.

Dan

Re: garbage collection

[Dave D. <dde...@es...>]

Ethan Merritt <merritt@u.washington.edu> writes:

> 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. 
>

I'm talking about the blocks themselves. The memory system maintains a
list of all blocks it has given out.

Whether the blocks are also linked into other lists (such as lists of
plots, or lists of udf, or whatever) is at a higher level.

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

Acutally, it's simpler than that. Can have one list of blocks. The
reference count is for "permanent" references. Transient blocks have a
zero ref. count. Then a gc happens at the end of every transaction,
and just reaps everything with 0 reference count.

using a block for a udf means that it picks up a ref count, and so the
memory manager doesn't reclaim the memory until the udf code drops the
reference.

Could also store a pointer to an "owner" block. Eg all blocks
allocated by a udf would reference that block. When the owner is
reaped, all the blocks marked as belonging to it get reaped.

The point is just that you do a little more work when you allocate a
block (to tell the memory manager what its properties are), but then
you don't have to bother finding blocks to free afterwards, because
the memory manager is doing the work of tracking stuff.



> 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.
>  

The difference is just that in a gc scheme, at the end of the plot you
just tell the memory manager to find and free all transient
blocks. You don't have to traverse data structures finding things.

Ditto for freeing a udf : you don't have to explicitly look along the
actions freeing string literals. You just drop the ref. count on the
primary udf block, and the memory manager does all the rest of the
work,


>> 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;
>

Don't know about you, but I would never write it in that order anyway.

> 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.  
>


In the proposed scheme, there is exactly one place where the blocks
are registered - in the memory manager. So there's only one place to
worry about getting it right. All the clients of the memory system
just ask for permanent or transient blocks.

But anyway, need to be careful when manipulating "permanent" blocks,
but for transient blocks, you never need to worry. The memory mananger
doesn't follow the clients unsafe links, but only follows its own
"safe" links.


dd
-- 
Dave Denholm              <dde...@es...>       http://www.esmertec.com

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

[Dave D. <dde...@es...>]

Ethan Merritt <merritt@u.washington.edu> writes:

> On Wednesday 06 July 2005 02:36 pm, Juergen Wieferink wrote:
>> But I have placed the "#ifdef" quite funny, did you notice?  
>
> I noticed. But then, we *had* been discussing whether to remove
> the conditional coding around string variables. I figured you
> were just off to a quick start :-) 
>

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.

I was wondering if there's a case for some sort of garbage
collection. Nothing sophisticated like moving memory around, updating
pointers, etc. Just simple reference counting (probably).
Perhaps something along the lines of:

allocated blocks are kept in linked lists : say a list of "permanent"
blocks (eg udf's), and a list of transient blocks (allocated during a
plot, for example). At end of a plot, everything in the transient list
is reclaimed.

items can have a reference count (eg for strings, rather than making a
copy, just increase the ref count on copy, and decrease it when the
string goes out of scope)

items can have cleanup routines.
 - eg for the recent issue with c++ destructors, the cleanup routine
 could be a c++ fn to cleanup objects
 - cleanup routine for an 'at' can in turn free string literals, etc.


Need to be a little careful with registration of non-trivial objects,
to make sure things are atomic wrt SIGINT. But I think it should be
do-able. All the allocation used to go through a common routine, so
it shouldn't be hard to intercept all allocs and frees.


dd
-- 
Dave Denholm              <dde...@es...>       http://www.esmertec.com