Thread: Memory-context slinging

Peter Eisentraut <peter_e@gmx.net> writes:
> Well, I would really like a way to register an entire memory context to be
> swept away by elog(ERROR). But that's probably too specialized.

Be careful what you ask for, you might get it ;-).

If you really truly want a context that's destroyed by elog(ERROR),
and not by any other event, then create it as a child of ErrorContext,
and that's what will happen.  But I suspect what you really want is a
context that's a child of TopTransactionContext and will go away at
either transaction commit or transaction abort.

A difficulty I have been noticing with some of these special-purpose
contexts is that you'd like to keep a pointer to them in some global
variable, but there's no easy way to determine whether that pointer is
currently valid (eg, whether the context has yet been created during
the current transaction).  This would be particularly nasty with
something like a child of ErrorContext --- has an error occurred since
you last used the context?

An example of how we currently have to get around this is
backend/commands/trigger.c's DeferredTriggerAbortXact() routine, which
xact.c has to know to call at transaction abort.  All it does is delete
trigger.c's private per-transaction context and reset the global pointer
to same.  Deleting the context is useless because that'd happen
automatically anyway when TopTransactionContext gets zapped.  So,
resetting the pointer is the only reason for this function to exist.

I've been toying with the notion of adding an optional "notify" callback
function to memory contexts.  If specified, it'd be called immediately
before resetting or deleting the context.  This'd give you a clean way
to reset pointers etc.

Comments anyone?
        regards, tom lane

Tom Lane writes:

> If you really truly want a context that's destroyed by elog(ERROR),
> and not by any other event, then create it as a child of ErrorContext,
> and that's what will happen.  But I suspect what you really want is a
> context that's a child of TopTransactionContext and will go away at
> either transaction commit or transaction abort.

No, actually I wanted the former. Notice how the configuration file is
first read in completely as a linked list. When something goes wrong
(garbage in the file) I need to free the list (and other things) before
calling elog. The alternative would be to create a separate memory context
for this processing, but that's more of a purity issue, it wouldn't make
it work better, AFAICT.

> A difficulty I have been noticing with some of these special-purpose
> contexts is that you'd like to keep a pointer to them in some global
> variable, but there's no easy way to determine whether that pointer is
> currently valid

Shouldn't it be NULL if it's not valid?


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane writes:
>> If you really truly want a context that's destroyed by elog(ERROR),
>> and not by any other event, then create it as a child of ErrorContext,
>> and that's what will happen.  But I suspect what you really want is a
>> context that's a child of TopTransactionContext and will go away at
>> either transaction commit or transaction abort.

> No, actually I wanted the former. Notice how the configuration file is
> first read in completely as a linked list. When something goes wrong
> (garbage in the file) I need to free the list (and other things) before
> calling elog.

Ah, I see: normal exit from the routine would be to copy the validated
list to permanent storage and free the temporary junk, but in case of
error exit you'd like to have the list thrown away automatically.

As a general thing I'd still recommend a child of a transaction context,
because that guarantees the temp storage will be cleaned up either way;
you can't accidentally forget about it.  However, for guc.c's purposes
a child of ErrorContext is probably better, because (I think) guc.c
needs to be able to run outside of any transaction.  You'll still leak
memory if you forget to free the context during normal exit, but at
least this technique handles the error-case cleanup for you.

> The alternative would be to create a separate memory context
> for this processing, but that's more of a purity issue, it wouldn't make
> it work better, AFAICT.

If you leak permanent storage upon error, that's a bug that needs to be
fixed, IMHO, not just a "purity" issue.

>> A difficulty I have been noticing with some of these special-purpose
>> contexts is that you'd like to keep a pointer to them in some global
>> variable, but there's no easy way to determine whether that pointer is
>> currently valid

> Shouldn't it be NULL if it's not valid?

Exactly.  The problem is how to make sure that that's true, if the
context gets discarded by general-purpose memory cleanup code that has
no idea you've got a pointer stashed away somewhere.  You need a hook
that can clear your pointer to NULL at the right time.  Basically this
mechanism could take the place of (some of the) hard-wired calls in
xact.c's abort processing, which would be a Good Thing --- consider
dynamically loaded code that has no prospect of being directly known
by xact.c.
        regards, tom lane