Fix race in SSI interaction with gin fast path.
authorThomas Munro <tmunro@postgresql.org>
Mon, 3 Jul 2023 04:20:01 +0000 (16:20 +1200)
committerThomas Munro <tmunro@postgresql.org>
Mon, 3 Jul 2023 21:07:31 +0000 (09:07 +1200)
The ginfast.c code previously checked for conflicts in before locking
the relevant buffer, leaving a window where a RW conflict could be
missed.  Re-order.

There was also a place where buffer ID and block number were confused
while trying to predicate-lock a page, noted by visual inspection.

Back-patch to all supported releases.  Fixes one more problem discovered
with the reproducer from bug #17949, in this case when Dmitry tried
other index types.

Reported-by: Artem Anisimov <artem.anisimov.255@gmail.com>
Reported-by: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/17949-a0f17035294a55e2%40postgresql.org

src/backend/access/gin/ginfast.c
src/backend/access/gin/ginget.c

index ca7d770d864f47f339bcbdadbde6495c34ab97a6..eb6c55483181d1f3ab236b2780ce7a1a4b9a3c36 100644 (file)
@@ -245,9 +245,10 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
        /*
         * An insertion to the pending list could logically belong anywhere in the
         * tree, so it conflicts with all serializable scans.  All scans acquire a
-        * predicate lock on the metabuffer to represent that.
+        * predicate lock on the metabuffer to represent that.  Therefore we'll
+        * check for conflicts in, but not until we have the page locked and are
+        * ready to modify the page.
         */
-       CheckForSerializableConflictIn(index, NULL, GIN_METAPAGE_BLKNO);
 
        if (collector->sumsize + collector->ntuples * sizeof(ItemIdData) > GinListPageSize)
        {
@@ -291,6 +292,8 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
                LockBuffer(metabuffer, GIN_EXCLUSIVE);
                metadata = GinPageGetMeta(metapage);
 
+               CheckForSerializableConflictIn(index, NULL, GIN_METAPAGE_BLKNO);
+
                if (metadata->head == InvalidBlockNumber)
                {
                        /*
@@ -353,6 +356,8 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
                char       *ptr;
                char       *collectordata;
 
+               CheckForSerializableConflictIn(index, NULL, GIN_METAPAGE_BLKNO);
+
                buffer = ReadBuffer(index, metadata->tail);
                LockBuffer(buffer, GIN_EXCLUSIVE);
                page = BufferGetPage(buffer);
index cb676a710fb09e2942daac740a5a95e14f08f528..1f0214498cd3e6d489d619bb710a144e6b39262d 100644 (file)
@@ -140,7 +140,9 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
         * Predicate lock entry leaf page, following pages will be locked by
         * moveRightIfItNeeded()
         */
-       PredicateLockPage(btree->index, stack->buffer, snapshot);
+       PredicateLockPage(btree->index,
+                                         BufferGetBlockNumber(stack->buffer),
+                                         snapshot);
 
        for (;;)
        {