> That's pretty similar to to PG_ENSURE_ERROR_CLEANUP, except that that is
> also called after FATAL errors. If we do this, we probably should try to
> come up with a easier to understand naming scheme. PG_TRY_WITH_CLEANUP
> vs. PG_ENSURE_ERROR_CLEANUP isn't very clear to a reader.
Yep.
>> Instead of doing anything with sigsetjmp(), this would just push a
>> frame onto a cleanup stack. We would call of those callbacks from
>> innermost to outermost before doing siglongjmp(). With this design,
>> we don't need any volatile-ization.
>
> On the other hand most of the callsites will need some extra state
> somewhere to keep track of what to undo. That's a bit of restructuring
> work too. And if the cleanup function needs to reference anything done
> in the TRY block, that state will need to be volatile too.
I don't think so.
>> Aside from any reduction in the need
>> for volatile, this might actually perform slightly better, because
>> sigsetjmp() is a system call on some platforms. There are probably
>> few cases where that actually matters, but the one in pq_getmessage(),
>> for example, might not be entirely discountable.
>
> Hm. How would you implement PG_TRY_WITH_CLEANUP without a sigsetjmp()?
Posit:
struct cleanup_entry { void (*callback)(void *); void *callback_arg; struct cleanup_entry *next;
};
cleanup_entry *cleanup_stack = NULL;
So PG_TRY_WITH_CLEANUP(_cb, _cb_arg) does (approximately) this:
{
cleanup_entry e;
cleanup_entry *orige;
e.callback = (_cb);
e.callback_arg = (_cb_arg);
e.next = cleanup_stack;
orige = cleanup_stack;
cleanup_stack = &e;
And when you PG_END_TRY_WITH_CLEANUP, we just do this:
cleanup_stack = orige;
}
And before doing sigsetjmp to the active handler, we run all the
functions on the stack. There shouldn't be any need for volatile; the
compiler has to know that once we make it possible to get at a pointer
to cb_arg via a global variable (cleanup_stack), any function we call
in another translation unit could decide to call that function and it
would need to see up-to-date values of everything cb_arg points to.
So before calling such a function it had better store that data to
memory, not just leave it lying around in a register somewhere.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
