LockHasWaiters() crashes on fast-path locks
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t54357psql -h localhost -U postgresBuilt from patchset v7 (message #7), September 20, 2026 at 06:14 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t54357_7 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t54357_7 && git checkout t54357_7Patchset v7 (message #7) is on t54357_7
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.
Thanks,
Satya
Attachments:
0001-lock-has-waiters-fast-path-fix.patchapplication/octet-stream; name=0001-lock-has-waiters-fast-path-fix.patchDownload+14-0
Hi,
On Wed, Mar 25, 2026 at 2:15 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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).
I see that this type of lock transfer is happening for prepared
statements (see AtPrepare_Locks [1]/* * If the local lock was taken via the fast-path, we need to move it * to the primary lock table, or just get a pointer to the existing * primary lock table entry if by chance it's already been * transferred. */ if (locallock->proclock == NULL)). 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?
[1]: /* * If the local lock was taken via the fast-path, we need to move it * to the primary lock table, or just get a pointer to the existing * primary lock table entry if by chance it's already been * transferred. */ if (locallock->proclock == NULL)
/*
* If the local lock was taken via the fast-path, we need to move it
* to the primary lock table, or just get a pointer to the existing
* primary lock table entry if by chance it's already been
* transferred.
*/
if (locallock->proclock == NULL)
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
Hi,
On Wed, Mar 25, 2026 at 3:06 PM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:
Hi,
On Wed, Mar 25, 2026 at 2:15 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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]/messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.com 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]: /messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.com
/messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.com
Thanks,
Satya
Attachments:
v2-0001-lock-has-waiters-fast-path-fix.patchapplication/octet-stream; name=v2-0001-lock-has-waiters-fast-path-fix.patchDownload+11-0
Updated the patch with a commit message.
On Wed, Mar 25, 2026 at 3:34 PM SATYANARAYANA NARLAPURAM <
satyanarlapuram@gmail.com> wrote:
Show quoted text
Hi,
On Wed, Mar 25, 2026 at 3:06 PM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:Hi,
On Wed, Mar 25, 2026 at 2:15 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> 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]
/messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.comThanks,
Satya
Hi,
On Thu, Mar 26, 2026 at 2:54 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:
Updated the patch with a commit message.
Thanks for sending the updated patch. It looks good to me. I verified
it with the other thread patch - it fixes the SEGV.
[1]: /messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.com
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
On Thu, Mar 26, 2026 at 7:06 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
Thanks for sending the updated patch. It looks good to me. I verified
it with the other thread patch - it fixes the SEGV.
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.
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.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On Tue, Jul 14, 2026 at 8:03 AM Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Mar 26, 2026 at 7:06 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> 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. /messages/by-id/CAHg+QDfdoR=7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q@mail.gmail.com
Please review the attached v4 patch.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
On Tue, Jul 14, 2026 at 11:02:56AM -0400, Robert Haas wrote:
On Thu, Mar 26, 2026 at 7:06 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:Thanks for sending the updated patch. It looks good to me. I verified
it with the other thread patch - it fixes the SEGV.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.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.
In terms of the core code, that's a true statement. Honestly, I would
be tempted to just add an assert and a comment and call it a day. I
don't see a strong need to support this case if we have nothing in
core to test it and nothing in core that needs it, and there are no
facts that we need to do anything. If there are optimizations in core
that could make use of it, and perhaps there are, that would make the
case of this proposal easier to support, at least that's my
impression. Without such cases in mind, this proposal feels mostly
like dead code to me.
Perhaps out-of-core code or extensions could make use of this API, but
in my experience this usually points at possible optimizations in the
backend core code. I'd suggest to look at that first, perhaps, if
you'd like to make LockHasWaiters() fast-path safe?
--
Michael
On Wed, Sep 02, 2026 at 08:53:47AM +0900, Michael Paquier wrote:
Perhaps out-of-core code or extensions could make use of this API, but
in my experience this usually points at possible optimizations in the
backend core code. I'd suggest to look at that first, perhaps, if
you'd like to make LockHasWaiters() fast-path safe?
Oh, I have missed the argument about autoprewarm and pg_prewarm on
this thread:
/messages/by-id/CALj2ACXXx9TwYzeP0cRnZKHz60yOYivcG7yiV=2FTC5chSVoOg@mail.gmail.com
Sorry about that. I'd be curious about a different case where this
could be used. Without the other proposal this feels a bit dead when
taken independently..
--
Michael
Hi,
On Tue, Sep 1, 2026 at 9:55 PM Michael Paquier <michael@paquier.xyz> wrote:
On Wed, Sep 02, 2026 at 08:53:47AM +0900, Michael Paquier wrote:
Perhaps out-of-core code or extensions could make use of this API, but
in my experience this usually points at possible optimizations in the
backend core code. I'd suggest to look at that first, perhaps, if
you'd like to make LockHasWaiters() fast-path safe?Oh, I have missed the argument about autoprewarm and pg_prewarm on
this thread:
/messages/by-id/CALj2ACXXx9TwYzeP0cRnZKHz60yOYivcG7yiV=2FTC5chSVoOg@mail.gmail.comSorry about that.
Thanks for taking a look at it.
I'd be curious about a different case where this
could be used.
Without the other proposal this feels a bit dead when
taken independently..
I haven't looked into other cases where this could be used independently.
For now, I will move this to the other thread as a 0001 patch and
close this thread's CF entry.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com