On Fri, Sep 3, 2010 at 6:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Now the HS case likewise appears to be set up so that the signal can
> only directly interrupt ProcWaitForSignal, so I think the core issue is
> whether any deadlock situations are possible. Given that this gets
> called from a low-level place like LockBufferForCleanup, I don't feel
> too comfortable about that. I certainly haven't seen any analysis or
> documentation of what locks can safely be held at that point.
> The deadlock checker only tries to take the LockMgr LWLocks, so
> extrapolating from whether it is safe to whether touching the
> ProcArrayLock is safe seems entirely unfounded.
>
> It might be worth pointing out here that LockBufferForCleanup is already
> known to be a risk factor for undetected deadlocks, even without HS in
> the picture, because of the possibility of deadlocks involving a chain
> of both heavyweight locks and LWLocks. Whether HS makes it materially
> worse may be something that we need field experience to determine.
OK, well, that does make it more clear what you're worried about, but
it seems that moving the work out of the signal handler isn't going to
solve this problem. The core issue appears to be whether the caller
might be holding a lock which another process might attempt to take
while already holding ProcArrayLock, or (in the case of a three or
more way deadlock) whether the caller might be holding a lock that
some other process could try to acquire after seizing ProcArrayLock.
As far as I can tell from looking through the code, the only locks we
ever acquire while holding ProcArrayLock are XidGenLock and
known_assigned_xids_lck (which is a spin lock). The latter is never
held more than VERY briefly. XidGenLock is generally also held only
briefly, not acquiring any other locks in the meantime, but
GetNewTransactionId() is not obviously (to me) safe.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company
