Re: Proposal for fixing intra-query memory leaks

On Sat, 29 Apr 2000, Tom Lane wrote:

> Background
> ----------
>
> We already do most of our memory allocation in "memory contexts", which
> are usually AllocSets as implemented by backend/utils/mmgr/aset.c.
> (Is there any value in allowing for other memory context types? We could

I have "aset" variant, that allow create and work with memory context in
a shared memory.

> save some cycles by getting rid of a level of indirection here.) What
> we need to do is create more contexts and define proper rules about when
> they can be freed.
>
> The basic operations on a memory context are:
>
> * create a context

A questuon, need we really any name for context? I mean an example
"CreateGlobalMemory(char *name)". Global memory routines expect that
a context name is static. It is a litle problem if contexts are created
dynamic. I suggest change it and allocate context name in context memory.

About blocks deallocation, now if context is destroyed for all blocks are
call free(). And if some context needs new block the aset.c call malloc().
What create a "BlockFreeList" with limited size (and allow set size for
this pool as new postmaster command line switch), and instead call the free()
for each block, remove a block (standard block, not a large) to this
BlockFreeList and instead the malloc() try first look at to this list?

> in that context. The MemoryContextSwitchTo() operation selects a new
> current context (and returns the previous context, so that the caller can
> restore the previous context before exiting).
>
> Note: there is no really good reason for pfree() to be tied to the current
> memory context; it ought to be possible to pfree() a chunk of memory no
> matter which context it was allocated from. Currently we cannot do that

I not sure if I understent here.

What the chunk->aset pointer in all chunks? It is pointer to a setData and
the AllocSetFree() need this pointer only. The AllocSetFree() not use full
context struct.

IMHO is very easy implement pfree() and repalloc() independent on
CurrentMemoryContext setting. The MemoryContextSwitchTo() needs palloc()
only. Or not?

> because of the possibility that there is more than one kind of memory
> context. If they were all AllocSets then the problem goes away, which is
> one reason I'd like to eliminate the provision for other kinds of
> contexts.

> Additions to the memory-context mechanism
> -----------------------------------------
>
> If we are going to have more contexts, we need more mechanism for keeping
> track of them; else we risk leaking whole contexts under error conditions.
> We can do this as follows:
>
> 1. There will be two kinds of contexts, "permanent" and "temporary".
> Permanent contexts are never reset or deleted except by explicit caller
> command (in practice, they probably won't ever be, period). There will
> not be very many of these --- perhaps only the existing TopMemoryContext
> and CacheMemoryContext. We should avoid having very much code run with
> CurrentMemoryContext pointing at a permanent context, since any forgotten
> palloc() represents a permanent memory leak.

And what you mean about total persistent contexts in shared memory?
I create for the QueryCache specific top context that is stored in the
QueryCache pool (shmem) and is independent on standard TopMemoryContext.

We probably don't know what we will need in future save to shared memory.
IMHO is good create some common mechanism for shmem, that allow use current
routines (an example copyObject()) for a work with shmem.

> 2. Temporary contexts are remembered by the context manager and are
> guaranteed to be deleted at transaction end. (If we ever have nested
> transactions, we'd probably want to tie each temporary context to a
> particular transaction, but for now that's not necessary.) Most activity
> will happen in temporary contexts.

What is the "context manager"?

Your suggestion/planns are very interesting. I agree that more contexts
(for error, query, transactions, ..etc.) is a good idea.

Karel