From e64f769fa001399ed962d59fb321ba9fee73676d Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Thu, 13 Aug 2026 14:01:19 +0300 Subject: [PATCH] Re-find the proclock after waiting instead of trusting a stale pointer PG16 counterpart of 28592521ad7 on patches18 (ORI-247, "PANIC: proclock table corrupted"). A plain cherry-pick does not apply: on PG18 LockAcquireExtended() releases the partition lock before WaitOnLock() and so the fix there re-takes it around the lookup, while on PG16 ProcSleep() holds it at entry and re-acquires it EXCLUSIVE before returning -- taking it again here would self-deadlock. The lookup is therefore a plain hash_search. Unlike PG17 there is no dontWait case in this block (a conditional acquisition returns earlier), so the re-find is unconditional. Also guard the INCONSISTENT prints, which can now see a proclock that is gone rather than merely one without our bit, and keep the local lock's cached pointers in step on the granted path. Verified the same way as the PG18 and PG17 fixes: with a probe reporting a proclock that was freed while we slept, the contended-upsert workload against orioledb hits it 161 times in 60 seconds on PG16 -- 161 reads of freed shared memory that this change no longer performs -- and runs with no panic and no assertion failure. --- src/backend/storage/lmgr/lock.c | 58 ++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index da6871fff8a..65486292391 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -1159,17 +1159,63 @@ LockAcquireExtended(const LOCKTAG *locktag, * done when the lock was granted to us --- see notes in WaitOnLock. */ + /* + * Whether the lock was granted has to be re-read from the lock table, + * not from the proclock pointer we captured before going to sleep. A + * wake-up driven by RemoveFromWaitQueue() -- which is how orioledb's + * oxid_notify_all() releases waiters, reporting PROC_WAIT_STATUS_OK + * afterwards -- runs CleanUpLock(), and that frees the proclock + * whenever it holds nothing else, which is always so for a pure + * waiter. Reading holdMask through the freed element yields whatever + * the next owner of that dynahash slot has put there; a stray set bit + * makes us believe in a grant that never happened, and the release + * that follows underflows the request counts and removes a proclock + * that is no longer in the table ("proclock table corrupted"). + * + * The lock itself can be gone too -- CleanUpLock() drops it once + * nRequested reaches zero -- so start from the locktag. Unlike the + * PG18 version of this fix, the partition lock is still held here: + * ProcSleep() takes it at entry and holds it at exit. + */ + { + PROCLOCKTAG proclocktag; + + lock = (LOCK *) hash_search_with_hash_value(LockMethodLockHash, + locktag, + hashcode, + HASH_FIND, + NULL); + proclock = NULL; + if (lock != NULL) + { + proclocktag.myLock = lock; + proclocktag.myProc = MyProc; + proclock = (PROCLOCK *) + hash_search_with_hash_value(LockMethodProcLockHash, + &proclocktag, + ProcLockHashCode(&proclocktag, + hashcode), + HASH_FIND, + NULL); + } + } + /* * Check the proclock entry status, in case something in the ipc - * communication doesn't work correctly. + * communication doesn't work correctly, or we were released from the + * queue by RemoveFromWaitQueue(). */ - if (!(proclock->holdMask & LOCKBIT_ON(lockmode))) + if (proclock == NULL || + !(proclock->holdMask & LOCKBIT_ON(lockmode))) { int i; AbortStrongLockAcquire(); - PROCLOCK_PRINT("LockAcquire: INCONSISTENT", proclock); - LOCK_PRINT("LockAcquire: INCONSISTENT", lock, lockmode); + if (proclock != NULL) + { + PROCLOCK_PRINT("LockAcquire: INCONSISTENT", proclock); + LOCK_PRINT("LockAcquire: INCONSISTENT", lock, lockmode); + } /* Should we retry ? */ LWLockRelease(partitionLock); /* @@ -1195,6 +1241,10 @@ LockAcquireExtended(const LOCKTAG *locktag, return LOCKACQUIRE_NOT_AVAIL; } + /* Keep the local lock's cached pointers in step. */ + locallock->lock = lock; + locallock->proclock = proclock; + PROCLOCK_PRINT("LockAcquire: granted", proclock); LOCK_PRINT("LockAcquire: granted", lock, lockmode); }