Make sure we only try to free snapshots that have been passed through

CopySnapshot, per Neil Conway.  Also add a comment about the assumption in
GetSnapshotData that the argument is statically allocated.

Also, fix some more typos in comments in snapmgr.c.

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 8603358ad96987e61616f83ec2d7154519f9be57..d07ebcb36b3ca6b7ae46c8f0b3ac09eabb48d27f 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -666,6 +666,9 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)
  *             RecentGlobalXmin: the global xmin (oldest TransactionXmin across all
  *                     running transactions, except those running LAZY VACUUM).  This is
  *                     the same computation done by GetOldestXmin(true, true).
+ *
+ * Note: this function should probably not be called with an argument that's
+ * not statically allocated (see xip allocation below).
  */
 Snapshot
 GetSnapshotData(Snapshot snapshot)

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 747e8c1e647b4742d9f7116e58b956789a2e1c3f..419c88a30debfd932c79640a9cf5044c39fcb7ce 100644 (file)
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -3,10 +3,9 @@
  *             PostgreSQL snapshot manager
  *
  * We keep track of snapshots in two ways: the "registered snapshots" list,
- * and the "active snapshot" stack.  All snapshots in any of them is supposed
- * to be in persistent memory.  When a snapshot is no longer in any of these
- * lists (tracked by separate refcounts of each snapshot), its memory can be
- * freed.
+ * and the "active snapshot" stack.  All snapshots in either of them live in
+ * persistent memory.  When a snapshot is no longer in any of these lists
+ * (tracked by separate refcounts on each snapshot), its memory can be freed.
  *
  * These arrangements let us reset MyProc->xmin when there are no snapshots
  * referenced by this transaction.  (One possible improvement would be to be
@@ -261,6 +260,7 @@ FreeSnapshot(Snapshot snapshot)
 {
        Assert(snapshot->regd_count == 0);
        Assert(snapshot->active_count == 0);
+       Assert(snapshot->copied);
 
        pfree(snapshot);
 }