From 37f2ef91d334161e88ea0c27f446fc13a831d60c Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Fri, 14 Aug 2026 02:11:53 -0400
Subject: [PATCH 1/2] Add an isolation test for the GIN posting tree relock
 race

ginVacuumPostingTreeLeaves() walks to the leftmost leaf of a posting tree, takes
the root under a share lock, drops it, re-takes it in exclusive mode, and never
re-checks GinPageIsLeaf().  A concurrent root split keeps the block number and
rewrites the page in place: ginPlaceToPage() with a null stack->parent memcpys a
freshly built internal page over the root, and GinInitPage built that page with
GIN_LEAF cleared.  So the page can be internal by the time the exclusive lock
arrives.  ginVacuumPostingTreeLeaf() then runs on an internal page, the
rightlink sweep ends immediately, and the vacuum reports success without having
visited a leaf.  ginbulkdelete() goes on to free the heap line pointers whose
index entries are still present.

The other two GIN sites that do the same share-then-exclusive relock both
re-check.  ginTraverseLock() carries the comment "But root can become non-leaf
during relock".  ginbulkdelete()'s own entry-tree descent tests
blkno == GIN_ROOT_BLKNO && !GinPageIsLeaf(page) and restarts.  This site does
neither.

This adds an injection point in the window where no buffer lock is held, and an
isolation test that drives the race deterministically: one session's VACUUM
stops there, a second session splits the posting tree root, and the vacuum is
then released onto a page that is no longer a leaf.

On an assert-enabled build the test does not reach its counts.  The first item
decoded from the internal page is (0,0), and ginVacuumItemPointers() trips
Assert(ItemPointerIsValid(pointer)):

  TRAP: failed Assert("ItemPointerIsValid(pointer)"), itemptr.h:105
    ginVacuumItemPointers
    ginVacuumPostingTreeLeaf
    ginbulkdelete

The backend dies and the rest of the isolation suite goes with it.  On a build
without asserts the vacuum runs to completion and the index is left disagreeing
with the heap: measured separately, an index scan reports 14000 rows for a
predicate where a sequential scan reports 13899.

The expected output records correct behavior, so the test fails until the
re-check is added rather than encoding the current behavior.  Verified both
ways: with a re-check applied locally the test passes with both counts at 13899,
and without it the assertion above fires.

The race window is a few instructions wide, which is why this needs an injection
point rather than a plain isolation spec.  Ordinary concurrent writers plus
autovacuum reach it without any of this machinery.

Introduced by fd83c83d0, which replaced the ginTraverseLock() call at this site
with a hand-written share-then-exclusive pair.  Reproduces on the 14, 18 and
master tips; the source is byte-identical on 15, 16, 17 and 19.
---
 src/backend/access/gin/ginvacuum.c            | 10 +++
 src/test/modules/injection_points/Makefile    |  1 +
 .../expected/gin_vacuum_relock.out            | 52 ++++++++++++
 src/test/modules/injection_points/meson.build |  1 +
 .../specs/gin_vacuum_relock.spec              | 84 +++++++++++++++++++
 5 files changed, 148 insertions(+)
 create mode 100644 src/test/modules/injection_points/expected/gin_vacuum_relock.out
 create mode 100644 src/test/modules/injection_points/specs/gin_vacuum_relock.spec

diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index 040f21a92..58a97e2d2 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -23,6 +23,7 @@
 #include "storage/lmgr.h"
 #include "storage/predicate.h"
 #include "storage/read_stream.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 struct GinVacuumState
@@ -398,6 +399,15 @@ ginVacuumPostingTreeLeaves(GinVacuumState *gvs, BlockNumber blkno)
 		if (GinPageIsLeaf(page))
 		{
 			LockBuffer(buffer, GIN_UNLOCK);
+
+			/*
+			 * No buffer lock is held here, so a concurrent insert can split
+			 * this page.  A root split rewrites the page in place and clears
+			 * GIN_LEAF, so the page may no longer be a leaf once the
+			 * exclusive lock is acquired below.
+			 */
+			INJECTION_POINT("gin-vacuum-posting-tree-relock", NULL);
+
 			LockBuffer(buffer, GIN_EXCLUSIVE);
 			break;
 		}
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 25a3ddd89..54e2857c4 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,6 +13,7 @@ REGRESS = injection_points hashagg reindex_conc vacuum
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
 ISOLATION = basic \
+	    gin_vacuum_relock \
 	    inplace \
 	    reindex_concurrently_deferred \
 	    repack \
diff --git a/src/test/modules/injection_points/expected/gin_vacuum_relock.out b/src/test/modules/injection_points/expected/gin_vacuum_relock.out
new file mode 100644
index 000000000..6c7c70edb
--- /dev/null
+++ b/src/test/modules/injection_points/expected/gin_vacuum_relock.out
@@ -0,0 +1,52 @@
+Parsed test spec with 2 sessions
+
+starting permutation: vacuum_gin split_root release recycle count_via_index count_via_heap
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step vacuum_gin: VACUUM gin_relock; <waiting ...>
+step split_root: INSERT INTO gin_relock
+					  SELECT g, ARRAY['public']
+					  FROM generate_series(20001, 28000) g;
+step release: 
+	SELECT injection_points_detach('gin-vacuum-posting-tree-relock');
+	SELECT injection_points_wakeup('gin-vacuum-posting-tree-relock');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step vacuum_gin: <... completed>
+step recycle: INSERT INTO gin_relock
+					  SELECT 900000 + g, ARRAY['recycled']
+					  FROM generate_series(1, 400) g;
+step count_via_index: 
+	SET enable_seqscan = off;
+	SET enable_indexscan = on;
+	SET enable_bitmapscan = on;
+	SELECT count(*) AS via_index FROM gin_relock WHERE tags @> ARRAY['public'];
+
+via_index
+---------
+    13899
+(1 row)
+
+step count_via_heap: 
+	SET enable_seqscan = on;
+	SET enable_indexscan = off;
+	SET enable_bitmapscan = off;
+	SELECT count(*) AS via_heap FROM gin_relock WHERE tags @> ARRAY['public'];
+
+via_heap
+--------
+   13899
+(1 row)
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index aaf0536ba..69a12d3cc 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -44,6 +44,7 @@ tests += {
   'isolation': {
     'specs': [
       'basic',
+      'gin_vacuum_relock',
       'inplace',
       'reindex_concurrently_deferred',
       'repack',
diff --git a/src/test/modules/injection_points/specs/gin_vacuum_relock.spec b/src/test/modules/injection_points/specs/gin_vacuum_relock.spec
new file mode 100644
index 000000000..cbd116e55
--- /dev/null
+++ b/src/test/modules/injection_points/specs/gin_vacuum_relock.spec
@@ -0,0 +1,84 @@
+# GIN VACUUM drops its share lock on a posting tree root and re-takes it in
+# exclusive mode without re-checking GinPageIsLeaf().  A concurrent root split
+# rewrites the page in place with GIN_LEAF cleared, so the page can be internal
+# by the time the exclusive lock arrives.  ginVacuumPostingTreeLeaf() then runs
+# on an internal page, the rightlink sweep ends at once, and the vacuum reports
+# success without having visited a leaf.  The heap pass frees the line pointers
+# whose index entries are still present, later rows recycle them, and an index
+# scan returns rows that a sequential scan over the same predicate does not.
+#
+# Both counts below must be 13899: 6000 rows loaded, 101 deleted, 8000 added.
+#
+# On an assert-enabled build this does not reach the counts.  The first item
+# decoded from the internal page is (0,0), and ginVacuumItemPointers() trips
+# Assert(ItemPointerIsValid(pointer)), so the backend dies and the rest of the
+# isolation suite goes with it.  On a build without asserts the vacuum runs to
+# completion and the counts diverge, via_index reporting 14000 against
+# via_heap's 13899.
+#
+# The other two GIN sites doing this share-then-exclusive relock both re-check.
+# ginTraverseLock() carries the comment "But root can become non-leaf during
+# relock", and ginbulkdelete()'s own entry-tree descent tests
+# blkno == GIN_ROOT_BLKNO && !GinPageIsLeaf(page) and restarts.
+
+setup
+{
+	CREATE EXTENSION injection_points;
+
+	CREATE TABLE gin_relock (id int, tags text[]);
+	INSERT INTO gin_relock
+		SELECT g, ARRAY['public'] FROM generate_series(1, 6000) g;
+	CREATE INDEX gin_relock_idx ON gin_relock USING gin (tags)
+		WITH (fastupdate = off);
+
+	-- The entries the VACUUM below is supposed to remove.
+	DELETE FROM gin_relock WHERE id BETWEEN 100 AND 200;
+}
+
+teardown
+{
+	DROP TABLE gin_relock;
+	DROP EXTENSION injection_points;
+}
+
+session s_vacuum
+setup
+{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('gin-vacuum-posting-tree-relock', 'wait');
+}
+# Stops in the window where no lock is held on the posting tree root.
+step vacuum_gin		{ VACUUM gin_relock; }
+# Recycles the line pointers the VACUUM freed.
+step recycle		{ INSERT INTO gin_relock
+					  SELECT 900000 + g, ARRAY['recycled']
+					  FROM generate_series(1, 400) g; }
+step count_via_index
+{
+	SET enable_seqscan = off;
+	SET enable_indexscan = on;
+	SET enable_bitmapscan = on;
+	SELECT count(*) AS via_index FROM gin_relock WHERE tags @> ARRAY['public'];
+}
+step count_via_heap
+{
+	SET enable_seqscan = on;
+	SET enable_indexscan = off;
+	SET enable_bitmapscan = off;
+	SELECT count(*) AS via_heap FROM gin_relock WHERE tags @> ARRAY['public'];
+}
+
+session s_writer
+# Splits the posting tree root while the VACUUM holds no lock on it.
+step split_root		{ INSERT INTO gin_relock
+					  SELECT g, ARRAY['public']
+					  FROM generate_series(20001, 28000) g; }
+# Detach before waking, so that a VACUUM which correctly notices the page is no
+# longer a leaf and restarts its descent does not stop here a second time.
+step release
+{
+	SELECT injection_points_detach('gin-vacuum-posting-tree-relock');
+	SELECT injection_points_wakeup('gin-vacuum-posting-tree-relock');
+}
+
+permutation vacuum_gin split_root release recycle count_via_index count_via_heap
-- 
2.53.0

