Re: [HACKERS] Proper cleanup at backend exit - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [HACKERS] Proper cleanup at backend exit
Date
Msg-id 199810012258.SAA19007@candle.pha.pa.us
Whole thread Raw
In response to Re: [HACKERS] Proper cleanup at backend exit  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
> Massimo Dal Zotto <dz@cs.unitn.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.)

Yes.  The old code atexit() was called for process exit and
shared-memory reset, and that caused problems as more and more cleanup
had to be done ONLY at shared memory reset time.  Having two separate
queues and proc names helped.

>
> 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.

That is why you need some type of global variable that sets the cleanup
the first time, and only once.  You could just call the proc_exit call,
and prevent it from scheduling two of the same exit functions.  That
would be cleaner.

>
> 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.


--
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026


pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Little parser question
Next
From: Tom Lane
Date:
Subject: Re: [HACKERS] It sorta works, but I'm confused about locking