Re: [HACKERS] Proper cleanup at backend exit

Massimo Dal Zotto <dz(at)cs(dot)unitn(dot)it> writes:
>> I have fixed several contributing bugs
>> (having to do with the limited size of the on_shmem_exit table),

> could you explain this to us ?

async.c tries to arrange to clean up its entries in pg_listener by
scheduling an exit callback routine via on_shmem_exit. (I think
Bruce or someone has been changing that stuff, and that on_shmem_exit
used to be called something else, but it's still basically an atexit-
like list of procedures to call.)

The trouble is that on_shmem_exit has a fixed-size, and not very large,
array for the callbacks. Try to register too many exit callbacks, and
after a while they stop getting registered.

I saw two problems associated with that. First, async.c was registering
a separate callback for each pg_listen entry it's ever made. That was
easily dealt with, since we now have an "unlisten all" subroutine:
I got rid of the per-listen callback and now register only a single
callback over the life of the backend. It just calls UnlistenAll.

Second, the main loop in postgres.c was miscoded so that every time you
had an error, it added a redundant FlushBufferPool callback to the list.
Make enough mistakes before you do your first listen, and you're still
out of luck. A simple reordering of the code fixes that. (While I was
at it I tried to improve the comments to make it clear where the main
loop *really* starts, so that future hackers won't misplace code in
PostgresMain the same way again...)

In the long term it might be a good idea to make on_shmem_exit construct
a linked list of registered callbacks (eg using Dllist) rather than
having a fixed maximum number of exit callbacks. But just at the moment
I don't think it's necessary, given these bug fixes --- and if we did
have a linked list in there, we'd still want to make these changes
because they would be memory leaks otherwise.

regards, tom lane

PS: I hope to check these changes in shortly.