From dabfbe45c63f165d94abb07814df536912c69a71 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 22 Dec 2025 19:12:19 +0200 Subject: [PATCH v2 1/5] Add sanity checks against using non-MVCC snapshots incorrectly These functions assumed that the active snapshot is a regular MVCC snapshot, but a historic MVCC snapshot can be pushed to the active stack too. These functions shouldn't be called when doing logical decoding, but better safe than sorry. --- contrib/amcheck/verify_heapam.c | 6 +++++- contrib/amcheck/verify_nbtree.c | 3 +++ src/backend/access/transam/parallel.c | 5 +++++ src/backend/catalog/pg_inherits.c | 4 ++++ src/backend/commands/async.c | 2 ++ src/backend/partitioning/partdesc.c | 8 +++++++- src/backend/utils/adt/xid8funcs.c | 2 ++ src/backend/utils/time/snapmgr.c | 2 ++ 8 files changed, 30 insertions(+), 2 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 130b3533463..2aae3119a7c 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -250,6 +250,7 @@ Datum verify_heapam(PG_FUNCTION_ARGS) { ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + Snapshot snap; HeapCheckContext ctx; Buffer vmbuffer = InvalidBuffer; Oid relid; @@ -310,7 +311,10 @@ verify_heapam(PG_FUNCTION_ARGS) * Any xmin newer than the xmin of our snapshot can't become all-visible * while we're running. */ - ctx.safe_xmin = GetTransactionSnapshot()->xmin; + snap = GetTransactionSnapshot(); + if (snap->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "verify_heapam() cannot be used with a non-MVCC snapshot"); + ctx.safe_xmin = snap->xmin; /* * If we report corruption when not examining some individual attribute, diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c index f91392a3a49..f9eb1af3bcf 100644 --- a/contrib/amcheck/verify_nbtree.c +++ b/contrib/amcheck/verify_nbtree.c @@ -440,6 +440,9 @@ bt_check_every_level(Relation rel, Relation heaprel, bool heapkeyspace, */ state->snapshot = RegisterSnapshot(GetTransactionSnapshot()); + if (state->snapshot->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "cannot check index consistency with a non-MVCC snapshot"); + /* * GetTransactionSnapshot() always acquires a new MVCC snapshot in * READ COMMITTED mode. A new snapshot is guaranteed to have all the diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index 642c61fc55c..423fce24248 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -229,6 +229,11 @@ InitializeParallelDSM(ParallelContext *pcxt) Snapshot transaction_snapshot = GetTransactionSnapshot(); Snapshot active_snapshot = GetActiveSnapshot(); + if (transaction_snapshot->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "cannot use parallel workers with non-MVCC transaction snapshot"); + if (active_snapshot->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "cannot use parallel workers with non-MVCC active snapshot"); + /* We might be running in a very short-lived memory context. */ oldcontext = MemoryContextSwitchTo(TopTransactionContext); diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index 929bb53b620..894656ec385 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -146,7 +146,11 @@ find_inheritance_children_extended(Oid parentrelId, bool omit_detached, Snapshot snap; xmin = HeapTupleHeaderGetXmin(inheritsTuple->t_data); + + /* XXX: what should we do with a non-MVCC snapshot? */ snap = GetActiveSnapshot(); + if (snap->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "cannot look up partition information with a non-MVCC snapshot"); if (!XidInMVCCSnapshot(xmin, snap)) { diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index eb86402cae4..0de8abeda28 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -1992,6 +1992,8 @@ asyncQueueProcessPageEntries(QueuePosition *current, alignas(AsyncQueueEntry) char local_buf[QUEUE_PAGESIZE]; char *local_buf_end = local_buf; + Assert(snapshot->snapshot_type == SNAPSHOT_MVCC); + slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, InvalidTransactionId); page_buffer = NotifyCtl->shared->page_buffer[slotno]; diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c index 985f48fc34d..e4ea63f212f 100644 --- a/src/backend/partitioning/partdesc.c +++ b/src/backend/partitioning/partdesc.c @@ -102,7 +102,13 @@ RelationGetPartitionDesc(Relation rel, bool omit_detached) Assert(TransactionIdIsValid(rel->rd_partdesc_nodetached_xmin)); activesnap = GetActiveSnapshot(); - if (!XidInMVCCSnapshot(rel->rd_partdesc_nodetached_xmin, activesnap)) + /* + * XXX: Not clear what we can do with a non-MVCC snapshot, so fall + * through to let RelationBuildPartitionDesc() handle it. (It + * currently just throws an error.) + */ + if (activesnap->snapshot_type == SNAPSHOT_MVCC && + !XidInMVCCSnapshot(rel->rd_partdesc_nodetached_xmin, activesnap)) return rel->rd_partdesc_nodetached; } diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index 4b3f7a69b3b..7b38eec6345 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -379,6 +379,8 @@ pg_current_snapshot(PG_FUNCTION_ARGS) cur = GetActiveSnapshot(); if (cur == NULL) elog(ERROR, "no active snapshot set"); + if (cur->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "pg_current_snapshot() cannot be used with a non-MVCC snapshot"); /* allocate */ nxip = cur->xcnt; diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 5af8326d5e8..f156845231b 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -1737,6 +1737,8 @@ SerializeSnapshot(Snapshot snapshot, char *start_address) { SerializedSnapshotData serialized_snapshot; + if (snapshot->snapshot_type != SNAPSHOT_MVCC) + elog(ERROR, "cannot serialize non-MVCC snapshot"); Assert(snapshot->subxcnt >= 0); /* Copy all required fields */ -- 2.47.3