On Wed, 2010-01-20 at 17:40 +0100, Andres Freund wrote:
> > > or similar things with LWLockAcquire in a signal handler
> >
> > [ grows visibly pale ] *Please* tell me we are not trying to take
> > locks in a signal handler. What happens if it interrupts code that
> > is already holding that lock?
> Yes the patch does that at two places.
I think it would be more sensible to discuss specific code and issues,
rather than have general discussions about various horrors.
You've already pointed out that I need to prevent multiple sigalrm
interrupts using boolean flags; I've already said that I would do that.
The use of locks themselves are clearly not a problem, since the
existing sigalrm handler takes LWlocks for deadlock detection. The
problem is just about being called multiple times.
The code in HoldingBufferPinThatDelaysRecovery() also needs protection
against being interrupted multiple times, but we should note that a
second signal of that type is not going to arrive from anywhere inside
the server and requires an explicit user action. The locking isn't
strictly necessary since the value is only read when the only process
that ever writes that value is sleeping on a semaphore. The single
integer value can always be read atomically anyway.
So I will remove the locking in XXXStartupBufferPinWaitBufId(), add in
the booleans and we're done.
-- Simon Riggs www.2ndQuadrant.com
