Skip site navigation (1) Skip section navigation (2)

Re: Memory leak in nodeAgg

From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Memory leak in nodeAgg
Date: 2007-08-07 00:13:35
Message-ID: 1186445615.16321.54.camel@dell.linuxdev.us.dell.com (view raw or flat)
Thread:
Lists: pgsql-patches
On Mon, 2007-08-06 at 18:52 -0400, Tom Lane wrote:
> Hmm.  Good catch, but I can't help wondering if this is just the tip
> of the iceberg.  Should *every* MemoryContextReset be
> MemoryContextResetAndDeleteChildren?

Yeah, the same thought occurred to me. Certainly having the current
behavior as the default is error-prone: it's quite easy to leak child
contexts on Reset. Perhaps we could redefine Reset to mean
ResetAndDeleteChildren, and add another name for the current Reset
functionality. ResetAndPreserveChildren, maybe?

> If we redefined MemoryContextReset to be the same as
> MemoryContextResetAndDeleteChildren, it'd be possible to keep the
> headers for child contexts in their parent context, thus easing
> traffic in TopMemoryContext, and perhaps saving a few pfree cycles
> when resetting the parent

The fact that MemoryContextCreate allocates the context header in
TopMemoryContext has always made me uneasy, so getting rid of that would
be nice. I wonder if there's not at least *one* place that depends on
the current Reset behavior, though...

> Anyone want to investigate what happens if we make MemoryContextReset
> the same as MemoryContextResetAndDeleteChildren?

Sure, I'll take a look, but I'll apply the attached patch in the mean
time (above cleanup is probably 8.4 material anyway).

-Neil



In response to

Responses

pgsql-patches by date

Next:From: Tom LaneDate: 2007-08-07 00:18:38
Subject: Re: Memory leak in nodeAgg
Previous:From: Tom LaneDate: 2007-08-06 22:52:13
Subject: Re: Memory leak in nodeAgg

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group