Updated the patch with a commit message. On Wed, Mar 25, 2026 at 3:34 PM SATYANARAYANA NARLAPURAM < [email protected]> wrote:
> Hi, > > On Wed, Mar 25, 2026 at 3:06 PM Bharath Rupireddy < > [email protected]> wrote: > >> Hi, >> >> On Wed, Mar 25, 2026 at 2:15 PM SATYANARAYANA NARLAPURAM >> <[email protected]> wrote: >> > >> > Hi Hackers, >> > >> > 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 locks (< ShareUpdateExclusiveLock) on >> relations may not be stored in the shared lock hash table, and the >> LOCALLOCK entry is left with lock = NULL and proclock = NULL in such a case. >> > >> > If LockHasWaiters() is called for such a lock, it dereferences those >> NULL pointers when it reads proclock->holdMask and lock->waitMask, causing >> a segfault. >> > >> > The only existing caller is lazy_truncate_heap() in VACUUM, which >> queries LockHasWaitersRelation(rel, AccessExclusiveLock). Since >> AccessExclusiveLock is the strongest lock level, it is never fast-pathed, >> so the bug has never been triggered in practice. However, any new caller >> that passes a weak lock mode, for example, checking whether a DDL is >> waiting on an AccessShareLock will crash. The fix is to transfer the lock >> to the main lock table before we access them. >> > >> > Attached a patch to address this issue. >> >> Nice find! It would be good to add a test case (perhaps in an existing >> test extension even though we may not commit it; it can act as a >> demo). >> > > Please refer the patches in the thread [2] below for a repro / use case. > > >> >> I see that this type of lock transfer is happening for prepared >> statements (see AtPrepare_Locks [1]). However, I see the proposed >> patch relying on lock == NULL for detecting whether the lock was >> acquired using fast-path. Although this looks correct because if the >> lock or proclock pointers are NULL, this identifies that the lock was >> taken using fast-path. But for consistency purposes, can we have the >> same check as that of AtPrepare_Locks? > > > Thank you for the review and code pointer, this is addressed now in v2 > patch, attached. > > [2] > https://www.postgresql.org/message-id/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmail.com > > > Thanks, > Satya >
v3-0001-Fix-LockHasWaiters-crash-for-fast-path-locks.patch
Description: Binary data
