Re: Thoughts about bug #3883 - Mailing list pgsql-hackers

I wrote:
> The issue Steven directly complained of is a potential for undetected
> deadlock via LockBufferForCleanup.  Ordinarily, buffer-level locks don't
> pose a deadlock risk because we don't hold one while trying to acquire
> another (except in UPDATE, which uses an ordering rule to avoid the
> risk).  The problem with LockBufferForCleanup is that it can be blocked
> by a mere pin, which another backend could well hold while trying to
> acquire a lock that will be blocked by VACUUM.

> There are a couple of migitating factors: first, patching TRUNCATE et al
> as suggested above will prevent the immediate case, and second, as of
> 8.3 this isn't a problem for autovacuum because of the facility for
> kicking autovacuum off a table if it's blocking someone else's lock
> request.

I did some experimentation and was dismayed to find that that last
statement isn't true --- at least on HPUX, the attempted SIGINT doesn't
cause autovacuum to cancel.  Some debugging and manual-reading led me to
the conclusion that a signal marked SA_RESTART is serviced but does not
cause semop() to return with EINTR; it just goes right back to waiting,
so we never get to the CHECK_FOR_INTERRUPTS call in PGSemaphoreLock.
Furthermore this appears to be the behavior expected by the Single Unix
Spec.  Cancellation does work on Linux, whose man page for semop() says
      semop()  is  never automatically restarted after being interrupted by a      signal handler, regardless of the
settingof the SA_RESTART  flag  when      establishing a signal handler.
 

But there is no such statement in the SUS.  Mac OS X seems to behave
like Linux, though this seems directly contrary to what it says in
its man pages.  That may mean that other BSDen do likewise.

If HPUX is the only platform that behaves like this, it might be that
we don't care enough to fix it, but I suspect that we might have a
problem on some other platforms too.

PGSemaphoreLock is called in three places: LWLockAcquire, which isn't
a problem because we don't want to accept cancel interrupts there
anyway; ProcWaitForSignal, which is the problem case; and ProcSleep
(ie, heavyweight-lock acquisition), which survives this case because it
sets lockAwaited, which is tested inside the interrupt service routine
via LockWaitCancel.  The simplest fix seems to be to invent an
additional flag variable "signalAwaited" which is set/cleared by
ProcWaitForSignal and checked by LockWaitCancel.  This would make
cancelling out of a ProcWaitForSignal call exactly analogous to
cancelling out of a heavyweight-lock acquisition.

My first inclination is to fix this in HEAD but not back branches,
because HEAD is the only branch where we really care that much about
autovacuum responding to SIGINT.  On the other hand, the undetected
deadlock is real enough in back branches, and maybe we should back-patch
it so that DBAs can get a manually issued signal to work.  Thoughts?
        regards, tom lane