Fix assertion failure in logical decoding.

Logical decoding set SnapshotData's regd_count field to avoid the
snapshot manager from prematurely freeing snapshots that are generated
by the decoding system. That was always an abuse of the field, as it was
never supposed to be used outside the snapshot manager. Commit 94028691
made snapshot manager's tracking of the snapshots smarter, and that scheme
fell apart. The snapshot manager got confused and hit the assertion, when
a snapshot that was marked with regd_count==1 was not found in the heap,
where the snapshot manager tracks registered the snapshots.

To fix, don't abuse the regd_count field like that. Logical decoding still
abuses the active_count field for similar purposes, but that's currently
harmless.

The assertion failure was first reported by Michael Paquier

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 20bb3b78e022fe689c8e652f4fbcb70ad6e7eee5..dc855830c4e468698da6eb57ebfa13713a47822a 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1188,8 +1188,8 @@ ReorderBufferCopySnap(ReorderBuffer *rb, Snapshot orig_snap,
        memcpy(snap, orig_snap, sizeof(SnapshotData));
 
        snap->copied = true;
-       snap->active_count = 0;
-       snap->regd_count = 1;
+       snap->active_count = 1;         /* mark as active so nobody frees it */
+       snap->regd_count = 0;
        snap->xip = (TransactionId *) (snap + 1);
 
        memcpy(snap->xip, orig_snap->xip, sizeof(TransactionId) * snap->xcnt);

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 82e7d986b42d65f60a22f42c8b114e9324183ac3..9b40bc8eca5de836fb12828c8f4bfbebde7b9035 100644 (file)
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -348,7 +348,7 @@ SnapBuildFreeSnapshot(Snapshot snap)
        Assert(snap->curcid == FirstCommandId);
        Assert(!snap->suboverflowed);
        Assert(!snap->takenDuringRecovery);
-       Assert(snap->regd_count == 1);
+       Assert(snap->regd_count == 0);
 
        /* slightly more likely, so it's checked even without c-asserts */
        if (snap->copied)
@@ -407,16 +407,16 @@ SnapBuildSnapDecRefcount(Snapshot snap)
        Assert(!snap->suboverflowed);
        Assert(!snap->takenDuringRecovery);
 
-       Assert(snap->regd_count == 1);
+       Assert(snap->regd_count == 0);
 
-       Assert(snap->active_count);
+       Assert(snap->active_count > 0);
 
        /* slightly more likely, so it's checked even without casserts */
        if (snap->copied)
                elog(ERROR, "cannot free a copied snapshot");
 
        snap->active_count--;
-       if (!snap->active_count)
+       if (snap->active_count == 0)
                SnapBuildFreeSnapshot(snap);
 }
 
@@ -495,7 +495,7 @@ SnapBuildBuildSnapshot(SnapBuild *builder, TransactionId xid)
        snapshot->copied = false;
        snapshot->curcid = FirstCommandId;
        snapshot->active_count = 0;
-       snapshot->regd_count = 1;       /* mark as registered so nobody frees it */
+       snapshot->regd_count = 0;
 
        return snapshot;
 }