Re: Memory leak in nodeAgg - Mailing list pgsql-patches

From Tom Lane
Subject Re: Memory leak in nodeAgg
Date
Msg-id 4068.1186440733@sss.pgh.pa.us
Whole thread Raw
In response to Memory leak in nodeAgg  (Neil Conway <neilc@samurai.com>)
Responses Re: Memory leak in nodeAgg  (Neil Conway <neilc@samurai.com>)
List pgsql-patches
Neil Conway <neilc@samurai.com> writes:
> Attached is a patch that fixes a gradual memory leak in ExecReScanAgg(),
> when the AGG_HASHED strategy is used:

Hmm.  Good catch, but I can't help wondering if this is just the tip
of the iceberg.  Should *every* MemoryContextReset be
MemoryContextResetAndDeleteChildren?

What this probably boils down to is whether there are cases where we
keep pointers to a sub-context in some place other than the parent
context.  I remember thinking there would be cases like that when I
proposed the current memory context API, but now I'm less sure that
it's a good idea.

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 (since we'd not bother explicitly releasing
the child headers).  But that would be a pain to undo if it turned out
wrong.

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

            regards, tom lane

pgsql-patches by date:

Previous
From: Neil Conway
Date:
Subject: Memory leak in nodeAgg
Next
From: Neil Conway
Date:
Subject: Re: Memory leak in nodeAgg