From 92e8f22277b7ea577c504b1f8be55fbe3bb7ab2b Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Thu, 13 Aug 2026 08:09:42 +0300 Subject: [PATCH] Re-find the proclock after waiting instead of trusting a stale pointer PANIC: proclock table corrupted, reported by Antithesis (ORI-247) on insert into txn0 as t (id, sk, val) values ($1,$2,$3) on conflict (id) do update set val = CONCAT(t.val, ',', $4) where t.id = $5 LockAcquireExtended() decides, after WaitOnLock() returns PROC_WAIT_STATUS_OK, whether the lock was granted by reading holdMask out of the proclock pointer it captured before going to sleep. That pointer is not guaranteed to still be ours. A waiter can be released by RemoveFromWaitQueue() rather than by the normal grant path -- orioledb's oxid_notify_all() does exactly that, and then reports PROC_WAIT_STATUS_OK, which is what the LOCKACQUIRE_NOT_AVAIL branch here already exists to absorb. RemoveFromWaitQueue() calls CleanUpLock(), and that deletes the proclock whenever it holds nothing else -- always the case for a pure waiter, e.g. the ShareLock a VirtualXactLock() sleeps on. The dynahash element then goes back on the freelist, so holdMask afterwards reads whatever the slot's next owner put there. Zero, usually, and the NOT_AVAIL branch does the right thing by accident. A stray set bit and we believe in a grant that never happened: GrantLockLocal() runs, and the LockRelease() that follows ungrants counts that were never incremented and removes a proclock that is no longer in the table. Re-find the lock and the proclock from the locktag under the partition lock, and decide from that. The lock itself can be gone too -- the same CleanUpLock() drops it once nRequested reaches zero -- so the lookup starts from the tag, not from the captured LOCK pointer. Measured with a probe that compared the captured pointer against a re-find: 313 reads of an already-freed proclock in two minutes of contended upserts against orioledb. Forcing the recycled slot to carry the lockmode bit turns the very first one into TRAP: failed Assert("(lock->nRequested > 0) && ..."), lock.c:1767 UnGrantLock / LockRelease / VirtualXactLock / ... table_tuple_insert_with_arbiter / ExecInsert which is the assert-build face of the reported PANIC. With this change the same workload takes the re-find path 163 times in 60 seconds and runs clean. PG16 and PG17 carry the same post-wait check and need the same fix. --- src/backend/storage/lmgr/lock.c | 68 ++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 0a0f4918db4..7280aa69423 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -1266,18 +1266,68 @@ LockAcquireExtended(const LOCKTAG *locktag, DeadLockReport(); /* DeadLockReport() will not return */ } - else if (!(proclock->holdMask & LOCKBIT_ON(lockmode))) + else { + PROCLOCKTAG proclocktag; + bool granted; + /* - * We've been removed from the queue without obtaining a lock. - * That's OK, we're going to return LOCKACQUIRE_NOT_AVAIL. + * 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. */ - AbortStrongLockAcquire(); - if (locallock->nLocks == 0) - RemoveLocalLock(locallock); - if (locallockp) - *locallockp = NULL; - return LOCKACQUIRE_NOT_AVAIL; + LWLockAcquire(partitionLock, LW_SHARED); + 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); + } + granted = (proclock != NULL && + (proclock->holdMask & LOCKBIT_ON(lockmode)) != 0); + LWLockRelease(partitionLock); + + if (!granted) + { + /* + * We've been removed from the queue without obtaining a lock. + * That's OK, we're going to return LOCKACQUIRE_NOT_AVAIL. + */ + AbortStrongLockAcquire(); + if (locallock->nLocks == 0) + RemoveLocalLock(locallock); + if (locallockp) + *locallockp = NULL; + return LOCKACQUIRE_NOT_AVAIL; + } + + /* Keep the local lock's cached pointers in step. */ + locallock->lock = lock; + locallock->proclock = proclock; } } else