Hi, On Tue, Jul 14, 2026 at 8:03 AM Robert Haas <[email protected]> wrote: > > On Thu, Mar 26, 2026 at 7:06 PM Bharath Rupireddy > <[email protected]> wrote: > > Thanks for sending the updated patch. It looks good to me. I verified > > it with the other thread patch - it fixes the SEGV.
Thanks Satya for the off-list discussion, and thanks Robert for the review. > To me, it seems like a bad idea for LockHasWaiters() to have the side > effect of adding entries to the main lock table. If nobody else has > moved our lock into the main lock table, it has no waiters. I think we > should just search LockMethodProcLockHash and see if we find anything, > and if not, return false without moving the lock. Agreed. A fast-path lock can only get a waiter if some backend asks for a conflicting lock, and in LockAcquireExtended() that backend first calls FastPathTransferRelationLocks() to move the matching fast-path locks into LockMethodLockHash and LockMethodProcLockHash before it waits. So if we look there and find nothing, the lock still has no waiters, and we can just return false without moving it. Attached v4 patch implements this. > Alternatively, if we don't need the functionality for anything, we > might just want LockHasWaiters() to assert > !EligibleForRelationFastPath(), and document the problem in a comment. The autoprewarm yielding to concurrent DDL work, which calls LockHasWaiters() for an AccessShareLock (which is fast-path), needs this fix. https://www.postgresql.org/message-id/flat/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmail.com Please review the attached v4 patch. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From 1d7f453406a0482e3f974beba4f1a56e6063a3a3 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Thu, 30 Jul 2026 20:52:41 +0000 Subject: [PATCH v4] Fix LockHasWaiters() crash for fast-path locks. LockHasWaiters() assumes that the LOCALLOCK's lock and proclock pointers are populated, but this is not the case for locks acquired via the fast-path optimization. Weak relation locks (those below ShareUpdateExclusiveLock, including AccessShareLock) are not stored in the shared lock hash table, leaving the LOCALLOCK entry with lock = NULL and proclock = NULL. If LockHasWaiters() is called for such a lock, it dereferences those NULL pointers when reading proclock->holdMask and lock->waitMask, causing a segfault. Having LockHasWaiters() transfer the lock into the main lock table with FastPathGetRelationLockEntry() would avoid the crash, but that gives a read-only check the side effect of adding entries to the main lock table. It is also unnecessary, because if nobody else has moved our lock into the main lock table, it has no waiters. Fix by looking up the main lock table when the LOCALLOCK pointers are NULL. If no entry is found, the lock is still held via the fast path and cannot have any waiters, so we return false without moving it. If an entry is found, some backend already transferred the lock, and we re-find the lock and proclock as LockRelease() does. Reported-by: Satyanarayana Narlapuram <[email protected]> Author: Bharath Rupireddy <[email protected]> Co-authored-by: Satyanarayana Narlapuram <[email protected]> Reviewed-by: Robert Haas <[email protected]> Discussion: https://www.postgresql.org/message-id/flat/CAHg%2BQDe_%3DZahnRx37bzrqYenKn_S5YDQ00fTfwe-ZUmjqO%3DqLg%40mail.gmail.com --- src/backend/storage/lmgr/lock.c | 40 ++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 0608eee9eb2..a57d185a77b 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -743,11 +743,45 @@ LockHasWaiters(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) LWLockAcquire(partitionLock, LW_SHARED); /* - * We don't need to re-find the lock or proclock, since we kept their - * addresses in the locallock table, and they couldn't have been removed - * while we were holding a lock on them. + * Normally we can rely on the lock and proclock addresses kept in the + * locallock table. But if the lock was acquired via the fast path, those + * pointers are NULL, because the lock was never entered in the shared + * lock table. A fast-path lock is a weak relation lock, and it can only + * gain a waiter if some backend requests a conflicting (strong) lock, and + * that request first moves all matching fast-path locks into the shared + * table (see FastPathTransferRelationLocks()). So if we still find no + * shared lock entry, the lock cannot have any waiters, and we return + * false without moving it. If another backend did move it, look up the + * lock and proclock here, the same way LockRelease() does. */ lock = locallock->lock; + if (!lock) + { + PROCLOCKTAG proclocktag; + + Assert(EligibleForRelationFastPath(locktag, lockmode)); + lock = (LOCK *) hash_search_with_hash_value(LockMethodLockHash, + locktag, + locallock->hashcode, + HASH_FIND, + NULL); + if (!lock) + { + /* Still fast-path only, so nobody could be waiting on it. */ + LWLockRelease(partitionLock); + return false; + } + locallock->lock = lock; + + proclocktag.myLock = lock; + proclocktag.myProc = MyProc; + locallock->proclock = (PROCLOCK *) hash_search(LockMethodProcLockHash, + &proclocktag, + HASH_FIND, + NULL); + if (!locallock->proclock) + elog(ERROR, "failed to re-find shared proclock object"); + } LOCK_PRINT("LockHasWaiters: found", lock, lockmode); proclock = locallock->proclock; PROCLOCK_PRINT("LockHasWaiters: found", proclock); -- 2.47.3
