MIME-Version: 1.0
References: 
 <CAHg+QDe_=ZahnRx37bzrqYenKn_S5YDQ00fTfwe-ZUmjqO=qLg@mail.gmail.com>
 <CALj2ACUKM1KQC=NHESOJw=ZgjAXgxapJfAEovKN0n3ZF8Csr5w@mail.gmail.com>
 <CAHg+QDc4f8hEEXODN5H=ETfOzg9QqaPW17=W_jYtXQUcAwZ84A@mail.gmail.com>
In-Reply-To: 
 <CAHg+QDc4f8hEEXODN5H=ETfOzg9QqaPW17=W_jYtXQUcAwZ84A@mail.gmail.com>
From: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Date: Thu, 26 Mar 2026 14:53:57 -0700
Message-ID: 
 <CAHg+QDfH5ZD7-wEdMc4bS4-A=N3bymy0ckHXUHOwrN-bE9AmXw@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="000000000000903b79064df46cdf"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHg%2BQDfH5ZD7-wEdMc4bS4-A%3DN3bymy0ckHXUHOwrN-bE9AmXw%40mail.gmail.com>
Precedence: bulk

--000000000000903b79064df46cdf
Content-Type: multipart/alternative; boundary="000000000000903b78064df46cdd"

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

Updated the patch with a commit message.

On Wed, Mar 25, 2026 at 3:34=E2=80=AFPM SATYANARAYANA NARLAPURAM <
satyanarlapuram@gmail.com> wrote:

> 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
>> 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 =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, causi=
ng
>> 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 loc=
k
>> 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%3D7iqEAvLW9qtzV0Sx1wp=
2FuALeamqcCdiVEmMF-Q%40mail.gmail.com
>
>
> Thanks,
> Satya
>

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

<div dir=3D"ltr">Updated the patch with a commit message.</div><br><div cla=
ss=3D"gmail_quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_at=
tr">On Wed, Mar 25, 2026 at 3:34=E2=80=AFPM SATYANARAYANA NARLAPURAM &lt;<a=
 href=3D"mailto:satyanarlapuram@gmail.com">satyanarlapuram@gmail.com</a>&gt=
; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px=
 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div di=
r=3D"ltr"><div>Hi,</div><br><div class=3D"gmail_quote"><div dir=3D"ltr" cla=
ss=3D"gmail_attr">On Wed, Mar 25, 2026 at 3:06=E2=80=AFPM Bharath Rupireddy=
 &lt;<a href=3D"mailto:bharath.rupireddyforpostgres@gmail.com" target=3D"_b=
lank">bharath.rupireddyforpostgres@gmail.com</a>&gt; wrote:<br></div><block=
quote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1=
px 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" target=3D"_blank">https://www.postgresql.org/mes=
sage-id/CAHg%2BQDfdoR%3D7iqEAvLW9qtzV0Sx1wp2FuALeamqcCdiVEmMF-Q%40mail.gmai=
l.com</a></div><div><br></div><div><br></div><div>Thanks,</div><div>Satya</=
div></div></div>
</blockquote></div>

--000000000000903b78064df46cdd--

--000000000000903b79064df46cdf
Content-Type: application/octet-stream;
	name="v3-0001-Fix-LockHasWaiters-crash-for-fast-path-locks.patch"
Content-Disposition: attachment;
	filename="v3-0001-Fix-LockHasWaiters-crash-for-fast-path-locks.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mn80f0kn0>
X-Attachment-Id: f_mn80f0kn0

RnJvbSAzNDlmM2ZhNTZhYmRhMWY5NDExNDAwYWU5YTA1ZWZhYmNiMjI3MGU4IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBTYXR5YSBOYXJsYXB1cmFtIDxzYXR5YW5hcmxhcHVyYW1AZ21h
aWwuY29tPgpEYXRlOiBUaHUsIDI2IE1hciAyMDI2IDIxOjQyOjE1ICswMDAwClN1YmplY3Q6IFtQ
QVRDSF0gRml4IExvY2tIYXNXYWl0ZXJzKCkgY3Jhc2ggZm9yIGZhc3QtcGF0aCBsb2NrcwoKTG9j
a0hhc1dhaXRlcnMoKSBhc3N1bWVzIHRoYXQgdGhlIExPQ0FMTE9DSydzIGxvY2sgYW5kIHByb2Ns
b2NrIHBvaW50ZXJzCmFyZSBwb3B1bGF0ZWQsIGJ1dCB0aGlzIGlzIG5vdCB0aGUgY2FzZSBmb3Ig
bG9ja3MgYWNxdWlyZWQgdmlhIHRoZQpmYXN0LXBhdGggb3B0aW1pemF0aW9uLiAgV2VhayByZWxh
dGlvbiBsb2NrcyAoPCBTaGFyZVVwZGF0ZUV4Y2x1c2l2ZUxvY2spLAppbmNsdWRpbmcgQWNjZXNz
U2hhcmVMb2NrLCBhcmUgbm90IHN0b3JlZCBpbiB0aGUgc2hhcmVkIGxvY2sgaGFzaCB0YWJsZSwK
bGVhdmluZyB0aGUgTE9DQUxMT0NLIGVudHJ5IHdpdGggbG9jayA9IE5VTEwgYW5kIHByb2Nsb2Nr
ID0gTlVMTC4KCklmIExvY2tIYXNXYWl0ZXJzKCkgaXMgY2FsbGVkIGZvciBzdWNoIGEgbG9jaywg
aXQgZGVyZWZlcmVuY2VzIHRob3NlIE5VTEwKcG9pbnRlcnMgd2hlbiByZWFkaW5nIHByb2Nsb2Nr
LT5ob2xkTWFzayBhbmQgbG9jay0+d2FpdE1hc2ssIGNhdXNpbmcgYQpzZWdmYXVsdC4KCgpGaXgg
YnkgY2hlY2tpbmcgd2hldGhlciB0aGUgTE9DQUxMT0NLIGhhcyBiZWVuIHBvcHVsYXRlZCBhbmQs
IGlmIG5vdCwKY2FsbGluZyBGYXN0UGF0aEdldFJlbGF0aW9uTG9ja0VudHJ5KCkgdG8gdHJhbnNm
ZXIgdGhlIGxvY2sgZnJvbSB0aGUKZmFzdC1wYXRoIGFycmF5cyBpbnRvIHRoZSBtYWluIGxvY2sg
dGFibGUuCi0tLQogc3JjL2JhY2tlbmQvc3RvcmFnZS9sbWdyL2xvY2suYyB8IDExICsrKysrKysr
KysrCiAxIGZpbGUgY2hhbmdlZCwgMTEgaW5zZXJ0aW9ucygrKQoKZGlmZiAtLWdpdCBhL3NyYy9i
YWNrZW5kL3N0b3JhZ2UvbG1nci9sb2NrLmMgYi9zcmMvYmFja2VuZC9zdG9yYWdlL2xtZ3IvbG9j
ay5jCmluZGV4IGQ5MzBjNjZjLi4zZjRkMzQzZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvc3Rv
cmFnZS9sbWdyL2xvY2suYworKysgYi9zcmMvYmFja2VuZC9zdG9yYWdlL2xtZ3IvbG9jay5jCkBA
IC03NDMsNiArNzQzLDE3IEBAIExvY2tIYXNXYWl0ZXJzKGNvbnN0IExPQ0tUQUcgKmxvY2t0YWcs
IExPQ0tNT0RFIGxvY2ttb2RlLCBib29sIHNlc3Npb25Mb2NrKQogCSAqLwogCXBhcnRpdGlvbkxv
Y2sgPSBMb2NrSGFzaFBhcnRpdGlvbkxvY2sobG9jYWxsb2NrLT5oYXNoY29kZSk7CiAKKwkvKgor
CSAqIElmIHRoZSBsb2NhbCBsb2NrIHdhcyB0YWtlbiB2aWEgdGhlIGZhc3QtcGF0aCwgd2UgbmVl
ZCB0byBtb3ZlIGl0CisJICogdG8gdGhlIHByaW1hcnkgbG9jayB0YWJsZSwgb3IganVzdCBnZXQg
YSBwb2ludGVyIHRvIHRoZSBleGlzdGluZworCSAqIHByaW1hcnkgbG9jayB0YWJsZSBlbnRyeSBp
ZiBieSBjaGFuY2UgaXQncyBhbHJlYWR5IGJlZW4gdHJhbnNmZXJyZWQuCisJICovCisJaWYgKGxv
Y2FsbG9jay0+cHJvY2xvY2sgPT0gTlVMTCkKKwl7CisJCWxvY2FsbG9jay0+cHJvY2xvY2sgPSBG
YXN0UGF0aEdldFJlbGF0aW9uTG9ja0VudHJ5KGxvY2FsbG9jayk7CisJCWxvY2FsbG9jay0+bG9j
ayA9IGxvY2FsbG9jay0+cHJvY2xvY2stPnRhZy5teUxvY2s7CisJfQorCiAJTFdMb2NrQWNxdWly
ZShwYXJ0aXRpb25Mb2NrLCBMV19TSEFSRUQpOwogCiAJLyoKLS0gCjIuNDMuMAoK
--000000000000903b79064df46cdf--