public inbox for [email protected]
help / color / mirror / Atom feedFrom: Noah Misch <[email protected]>
To: Alexander Lakhin <[email protected]>
Cc: Nitin Motiani <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Cc: Smolkin Grigory <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: race condition in pg_class
Date: Mon, 21 Oct 2024 20:52:15 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAH5HC96ASNkXyYVEG7pO98qr1WARGTohxGjVpBsu4c3hkM_OcA@mail.gmail.com>
<[email protected]>
<CAH5HC96ZwrjKF-RBijntw9uX2UzUtEKC3A7cxB_Xh8arVuKhKw@mail.gmail.com>
<[email protected]>
<CAH5HC974+yieU=uTWPtpY1+irUZAY+5GiGKiXjfKhLzo+jsE+g@mail.gmail.com>
<[email protected]>
<CAH5HC94xjj7JvRXD4LHVMgv=qVxuMm0On+2OvrGqeON4U-FTuQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
On Mon, Oct 21, 2024 at 10:00:00PM +0300, Alexander Lakhin wrote:
> Please look at an anomaly introduced with a07e03fd8.
> With the attached modification for intra-grant-inplace.spec, running this
> test triggers a Valgrind-detected error for me:
> ==00:00:00:09.624 319033== Conditional jump or move depends on uninitialised value(s)
> ==00:00:00:09.624 319033== at 0x25D120: DoesMultiXactIdConflict (heapam.c:7373)
> ==00:00:00:09.624 319033== by 0x25B45A: heap_inplace_lock (heapam.c:6265)
> ==00:00:00:09.624 319033== by 0x27D8CB: systable_inplace_update_begin (genam.c:867)
> ==00:00:00:09.624 319033== by 0x33F717: index_update_stats (index.c:2856)
> ==00:00:00:09.624 319033== by 0x33FEE2: index_build (index.c:3106)
> ==00:00:00:09.625 319033== by 0x33C7D3: index_create (index.c:1276)
> ==00:00:00:09.625 319033== by 0x451000: DefineIndex (indexcmds.c:1216)
> ==00:00:00:09.625 319033== by 0x48D091: ATExecAddIndex (tablecmds.c:9156)
> ==00:00:00:09.625 319033== by 0x483F8E: ATExecCmd (tablecmds.c:5302)
> ==00:00:00:09.625 319033== by 0x483877: ATRewriteCatalogs (tablecmds.c:5186)
> ==00:00:00:09.625 319033== by 0x482B9A: ATController (tablecmds.c:4741)
> ==00:00:00:09.625 319033== by 0x4827A1: AlterTable (tablecmds.c:4387)
> ==00:00:00:09.625 319033==
Thanks.
> Perhaps current_is_member in heap_inplace_lock() should be initialized
> before the DoesMultiXactIdConflict() call as in other places...
heap_inplace_lock() ignores current_is_member after computing it, so let's
just pass NULL, as attached.
Author: Noah Misch <[email protected]>
Commit: Noah Misch <[email protected]>
Stop reading uninitialized memory in heap_inplace_lock().
Stop computing a never-used value. This removes the read; the read had
no functional implications. Back-patch to v12, like commit
a07e03fd8fa7daf4d1356f7cb501ffe784ea6257.
Reported by Alexander Lakhin. Reviewed by FIXME.
Discussion: https://postgr.es/m/[email protected]
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index da5e656..82a0492 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6260,10 +6260,9 @@ heap_inplace_lock(Relation relation,
LockTupleMode lockmode = LockTupleNoKeyExclusive;
MultiXactStatus mxact_status = MultiXactStatusNoKeyUpdate;
int remain;
- bool current_is_member;
if (DoesMultiXactIdConflict((MultiXactId) xwait, infomask,
- lockmode, ¤t_is_member))
+ lockmode, NULL))
{
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
ret = false;
diff --git a/src/test/isolation/expected/intra-grant-inplace.out b/src/test/isolation/expected/intra-grant-inplace.out
index b5fe8b0..4e9695a 100644
--- a/src/test/isolation/expected/intra-grant-inplace.out
+++ b/src/test/isolation/expected/intra-grant-inplace.out
@@ -63,6 +63,30 @@ step addk2: ALTER TABLE intra_grant_inplace ADD PRIMARY KEY (c); <waiting ...>
step r3: ROLLBACK;
step addk2: <... completed>
+starting permutation: b3 sfnku3 keyshr5 addk2 r3
+step b3: BEGIN ISOLATION LEVEL READ COMMITTED;
+step sfnku3:
+ SELECT relhasindex FROM pg_class
+ WHERE oid = 'intra_grant_inplace'::regclass FOR NO KEY UPDATE;
+
+relhasindex
+-----------
+f
+(1 row)
+
+step keyshr5:
+ SELECT relhasindex FROM pg_class
+ WHERE oid = 'intra_grant_inplace'::regclass FOR KEY SHARE;
+
+relhasindex
+-----------
+f
+(1 row)
+
+step addk2: ALTER TABLE intra_grant_inplace ADD PRIMARY KEY (c); <waiting ...>
+step r3: ROLLBACK;
+step addk2: <... completed>
+
starting permutation: b2 sfnku2 addk2 c2
step b2: BEGIN;
step sfnku2:
diff --git a/src/test/isolation/specs/intra-grant-inplace.spec b/src/test/isolation/specs/intra-grant-inplace.spec
index 2992c85..9936d38 100644
--- a/src/test/isolation/specs/intra-grant-inplace.spec
+++ b/src/test/isolation/specs/intra-grant-inplace.spec
@@ -96,6 +96,14 @@ permutation
addk2(r3)
r3
+# reproduce bug in DoesMultiXactIdConflict() call
+permutation
+ b3
+ sfnku3
+ keyshr5
+ addk2(r3)
+ r3
+
# same-xact rowmark
permutation
b2
Attachments:
[text/plain] inplace129-DoesMultiXactIdConflict-v1.patch (2.5K, ../[email protected]/2-inplace129-DoesMultiXactIdConflict-v1.patch)
download | inline diff:
Author: Noah Misch <[email protected]>
Commit: Noah Misch <[email protected]>
Stop reading uninitialized memory in heap_inplace_lock().
Stop computing a never-used value. This removes the read; the read had
no functional implications. Back-patch to v12, like commit
a07e03fd8fa7daf4d1356f7cb501ffe784ea6257.
Reported by Alexander Lakhin. Reviewed by FIXME.
Discussion: https://postgr.es/m/[email protected]
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index da5e656..82a0492 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6260,10 +6260,9 @@ heap_inplace_lock(Relation relation,
LockTupleMode lockmode = LockTupleNoKeyExclusive;
MultiXactStatus mxact_status = MultiXactStatusNoKeyUpdate;
int remain;
- bool current_is_member;
if (DoesMultiXactIdConflict((MultiXactId) xwait, infomask,
- lockmode, ¤t_is_member))
+ lockmode, NULL))
{
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
ret = false;
diff --git a/src/test/isolation/expected/intra-grant-inplace.out b/src/test/isolation/expected/intra-grant-inplace.out
index b5fe8b0..4e9695a 100644
--- a/src/test/isolation/expected/intra-grant-inplace.out
+++ b/src/test/isolation/expected/intra-grant-inplace.out
@@ -63,6 +63,30 @@ step addk2: ALTER TABLE intra_grant_inplace ADD PRIMARY KEY (c); <waiting ...>
step r3: ROLLBACK;
step addk2: <... completed>
+starting permutation: b3 sfnku3 keyshr5 addk2 r3
+step b3: BEGIN ISOLATION LEVEL READ COMMITTED;
+step sfnku3:
+ SELECT relhasindex FROM pg_class
+ WHERE oid = 'intra_grant_inplace'::regclass FOR NO KEY UPDATE;
+
+relhasindex
+-----------
+f
+(1 row)
+
+step keyshr5:
+ SELECT relhasindex FROM pg_class
+ WHERE oid = 'intra_grant_inplace'::regclass FOR KEY SHARE;
+
+relhasindex
+-----------
+f
+(1 row)
+
+step addk2: ALTER TABLE intra_grant_inplace ADD PRIMARY KEY (c); <waiting ...>
+step r3: ROLLBACK;
+step addk2: <... completed>
+
starting permutation: b2 sfnku2 addk2 c2
step b2: BEGIN;
step sfnku2:
diff --git a/src/test/isolation/specs/intra-grant-inplace.spec b/src/test/isolation/specs/intra-grant-inplace.spec
index 2992c85..9936d38 100644
--- a/src/test/isolation/specs/intra-grant-inplace.spec
+++ b/src/test/isolation/specs/intra-grant-inplace.spec
@@ -96,6 +96,14 @@ permutation
addk2(r3)
r3
+# reproduce bug in DoesMultiXactIdConflict() call
+permutation
+ b3
+ sfnku3
+ keyshr5
+ addk2(r3)
+ r3
+
# same-xact rowmark
permutation
b2
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: race condition in pg_class
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox