MIME-Version: 1.0
References: 
 <CAHg+QDe_=ZahnRx37bzrqYenKn_S5YDQ00fTfwe-ZUmjqO=qLg@mail.gmail.com>
 <CALj2ACUKM1KQC=NHESOJw=ZgjAXgxapJfAEovKN0n3ZF8Csr5w@mail.gmail.com>
In-Reply-To: 
 <CALj2ACUKM1KQC=NHESOJw=ZgjAXgxapJfAEovKN0n3ZF8Csr5w@mail.gmail.com>
From: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Date: Wed, 25 Mar 2026 15:34:13 -0700
Message-ID: 
 <CAHg+QDc4f8hEEXODN5H=ETfOzg9QqaPW17=W_jYtXQUcAwZ84A@mail.gmail.com>
Subject: Re: LockHasWaiters() crashes on fast-path locks
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000b7c63c064de0deb4"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHg%2BQDc4f8hEEXODN5H%3DETfOzg9QqaPW17%3DW_jYtXQUcAwZ84A%40mail.gmail.com>
Precedence: bulk

--000000000000b7c63c064de0deb4
Content-Type: multipart/alternative; boundary="000000000000b7c63b064de0deb2"

--000000000000b7c63b064de0deb2
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Wed, Mar 25, 2026 at 3:06=E2=80=AFPM Bharath Rupireddy <
bharath.rupireddyforpostgres@gmail.com> wrote:

> Hi,
>
> On Wed, Mar 25, 2026 at 2:15=E2=80=AFPM SATYANARAYANA NARLAPURAM
> <satyanarlapuram@gmail.com> wrote:
> >
> > Hi Hackers,
> >
> > LockHasWaiters() assumes that the LOCALLOCK's lock and proclock pointer=
s
> 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 =3D NULL and proclock =3D 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, causin=
g
> 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 =3D=3D 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%3D7iqEAvLW9qtzV0Sx1wp2F=
uALeamqcCdiVEmMF-Q%40mail.gmail.com


Thanks,
Satya

--000000000000b7c63b064de0deb2
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Hi,</div><br><div class=3D"gmail_quote gmail_quote_co=
ntainer"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, Mar 25, 2026 at 3:06=
=E2=80=AFPM Bharath Rupireddy &lt;<a href=3D"mailto:bharath.rupireddyforpos=
tgres@gmail.com">bharath.rupireddyforpostgres@gmail.com</a>&gt; wrote:<br><=
/div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bo=
rder-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Wed, Mar 25, 2026 at 2:15=E2=80=AFPM SATYANARAYANA NARLAPURAM<br>
&lt;<a href=3D"mailto:satyanarlapuram@gmail.com" target=3D"_blank">satyanar=
lapuram@gmail.com</a>&gt; wrote:<br>
&gt;<br>
&gt; Hi Hackers,<br>
&gt;<br>
&gt; LockHasWaiters() assumes that the LOCALLOCK&#39;s lock and proclock po=
inters are populated, but this is not the case for locks acquired via the f=
ast-path optimization. Weak locks (&lt; ShareUpdateExclusiveLock) on relati=
ons may not be stored in the shared lock hash table, and the LOCALLOCK entr=
y is left with lock =3D NULL and proclock =3D NULL in such a case.<br>
&gt;<br>
&gt; If LockHasWaiters() is called for such a lock, it dereferences those N=
ULL pointers when it reads proclock-&gt;holdMask and lock-&gt;waitMask, cau=
sing a segfault.<br>
&gt;<br>
&gt; The only existing caller is lazy_truncate_heap() in VACUUM, which quer=
ies LockHasWaitersRelation(rel, AccessExclusiveLock). Since AccessExclusive=
Lock is the strongest lock level, it is never fast-pathed, so the bug has n=
ever been triggered in practice. However, any new caller that passes a weak=
 lock mode, for example, checking whether a DDL is waiting on an AccessShar=
eLock will crash. The fix is to transfer the lock to the main lock table be=
fore we access them.<br>
&gt;<br>
&gt; Attached a patch to address this issue.<br>
<br>
Nice find! It would be good to add a test case (perhaps in an existing<br>
test extension even though we may not commit it; it can act as a<br>
demo).<br></blockquote><div><br></div><div>Please refer the=C2=A0patches in=
 the thread [2] below for a repro / use case.</div><div>=C2=A0</div><blockq=
uote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1p=
x solid rgb(204,204,204);padding-left:1ex">
<br>
I see that this type of lock transfer is happening for prepared<br>
statements (see AtPrepare_Locks [1]). However, I see the proposed<br>
patch relying on lock =3D=3D NULL for detecting whether the lock was<br>
acquired using fast-path. Although this looks correct because if the<br>
lock or proclock pointers are NULL, this identifies that the lock was<br>
taken using fast-path. But for consistency purposes, can we have the<br>
same check as that of AtPrepare_Locks?</blockquote><div><br></div><div>Than=
k=C2=A0you for the review and code pointer, this is addressed now in v2 pat=
ch, attached.=C2=A0</div><div><br></div><div>[2]=C2=A0<a href=3D"https://ww=
w.postgresql.org/message-id/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCd=
iVEmMF-Q%40mail.gmail.com">https://www.postgresql.org/message-id/CAHg%2BQDf=
doR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmail.com</a></div><di=
v><br></div><div><br></div><div>Thanks,</div><div>Satya</div></div></div>

--000000000000b7c63b064de0deb2--

--000000000000b7c63c064de0deb4
Content-Type: application/octet-stream;
	name="v2-0001-lock-has-waiters-fast-path-fix.patch"
Content-Disposition: attachment;
	filename="v2-0001-lock-has-waiters-fast-path-fix.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mn6me8wu0>
X-Attachment-Id: f_mn6me8wu0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3N0b3JhZ2UvbG1nci9sb2NrLmMgYi9zcmMvYmFja2Vu
ZC9zdG9yYWdlL2xtZ3IvbG9jay5jCmluZGV4IGQ5MzBjNjZjLi4zZjRkMzQzZCAxMDA2NDQKLS0t
IGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9sbWdyL2xvY2suYworKysgYi9zcmMvYmFja2VuZC9zdG9y
YWdlL2xtZ3IvbG9jay5jCkBAIC03NDMsNiArNzQzLDE3IEBAIExvY2tIYXNXYWl0ZXJzKGNvbnN0
IExPQ0tUQUcgKmxvY2t0YWcsIExPQ0tNT0RFIGxvY2ttb2RlLCBib29sIHNlc3Npb25Mb2NrKQog
CSAqLwogCXBhcnRpdGlvbkxvY2sgPSBMb2NrSGFzaFBhcnRpdGlvbkxvY2sobG9jYWxsb2NrLT5o
YXNoY29kZSk7CiAKKwkvKgorCSAqIElmIHRoZSBsb2NhbCBsb2NrIHdhcyB0YWtlbiB2aWEgdGhl
IGZhc3QtcGF0aCwgd2UgbmVlZCB0byBtb3ZlIGl0CisJICogdG8gdGhlIHByaW1hcnkgbG9jayB0
YWJsZSwgb3IganVzdCBnZXQgYSBwb2ludGVyIHRvIHRoZSBleGlzdGluZworCSAqIHByaW1hcnkg
bG9jayB0YWJsZSBlbnRyeSBpZiBieSBjaGFuY2UgaXQncyBhbHJlYWR5IGJlZW4gdHJhbnNmZXJy
ZWQuCisJICovCisJaWYgKGxvY2FsbG9jay0+cHJvY2xvY2sgPT0gTlVMTCkKKwl7CisJCWxvY2Fs
bG9jay0+cHJvY2xvY2sgPSBGYXN0UGF0aEdldFJlbGF0aW9uTG9ja0VudHJ5KGxvY2FsbG9jayk7
CisJCWxvY2FsbG9jay0+bG9jayA9IGxvY2FsbG9jay0+cHJvY2xvY2stPnRhZy5teUxvY2s7CisJ
fQorCiAJTFdMb2NrQWNxdWlyZShwYXJ0aXRpb25Mb2NrLCBMV19TSEFSRUQpOwogCiAJLyoK
--000000000000b7c63c064de0deb4--