Fix a couple of snapshot management bugs in the new ResourceOwner world:

non-writable large objects need to have their snapshots registered on the
transaction resowner, not the current portal's, because it must persist until
the large object is closed (which the portal does not).  Also, ensure that the
serializable snapshot is recorded by the transaction resource owner too, even
when a subtransaction has changed the current resource owner before
serializable is taken.

Per bug reports from Pavan Deolasee.

3 files changed, 73 insertions, 18 deletions

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b61fe41083..c5a1b33a9e 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.269 2008/11/19 10:34:50 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.270 2008/12/04 14:51:02 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1667,6 +1667,9 @@ CommitTransaction(void)
 	/* Clean up the relation cache */
 	AtEOXact_RelationCache(true);
 
+	/* Clean up the snapshot manager */
+	AtEarlyCommit_Snapshot();
+
 	/*
 	 * Make catalog changes visible to all backends.  This has to happen after
 	 * relcache references are dropped (see comments for
@@ -1906,6 +1909,9 @@ PrepareTransaction(void)
 	/* Clean up the relation cache */
 	AtEOXact_RelationCache(true);
 
+	/* Clean up the snapshot manager */
+	AtEarlyCommit_Snapshot();
+
 	/* notify and flatfiles don't need a postprepare call */
 
 	PostPrepare_PgStat();
diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c
index 3936260e6c..14f101adb3 100644
--- a/src/backend/storage/large_object/inv_api.c
+++ b/src/backend/storage/large_object/inv_api.c
@@ -24,7 +24,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.135 2008/11/02 01:45:28 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/large_object/inv_api.c,v 1.136 2008/12/04 14:51:02 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -247,7 +247,13 @@ inv_open(Oid lobjId, int flags, MemoryContext mcxt)
 	}
 	else if (flags & INV_READ)
 	{
-		retval->snapshot = RegisterSnapshot(GetActiveSnapshot());
+		/*
+		 * We must register the snapshot in TopTransaction's resowner,
+		 * because it must stay alive until the LO is closed rather than until
+		 * the current portal shuts down.
+		 */
+		retval->snapshot = RegisterSnapshotOnOwner(GetActiveSnapshot(),
+												   TopTransactionResourceOwner);
 		retval->flags = IFS_RDLOCK;
 	}
 	else
@@ -270,8 +276,11 @@ void
 inv_close(LargeObjectDesc *obj_desc)
 {
 	Assert(PointerIsValid(obj_desc));
+
 	if (obj_desc->snapshot != SnapshotNow)
-		UnregisterSnapshot(obj_desc->snapshot);
+		UnregisterSnapshotFromOwner(obj_desc->snapshot,
+									TopTransactionResourceOwner);
+
 	pfree(obj_desc);
 }
 
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 1107abdf27..d37dc5df84 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.7 2008/11/25 20:28:29 alvherre Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/time/snapmgr.c,v 1.8 2008/12/04 14:51:02 alvherre Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -136,7 +136,8 @@ GetTransactionSnapshot(void)
 		 */
 		if (IsXactIsoLevelSerializable)
 		{
-			CurrentSnapshot = RegisterSnapshot(CurrentSnapshot);
+			CurrentSnapshot = RegisterSnapshotOnOwner(CurrentSnapshot,
+													  TopTransactionResourceOwner);
 			registered_serializable = true;
 		}
 
@@ -345,14 +346,27 @@ ActiveSnapshotSet(void)
 
 /*
  * RegisterSnapshot
- * 		Register a snapshot as being in use
+ * 		Register a snapshot as being in use by the current resource owner
  *
  * If InvalidSnapshot is passed, it is not registered.
  */
 Snapshot
 RegisterSnapshot(Snapshot snapshot)
 {
-	Snapshot	snap;
+	if (snapshot == InvalidSnapshot)
+		return InvalidSnapshot;
+
+	return RegisterSnapshotOnOwner(snapshot, CurrentResourceOwner);
+}
+
+/*
+ * RegisterSnapshotOnOwner
+ * 		As above, but use the specified resource owner
+ */
+Snapshot
+RegisterSnapshotOnOwner(Snapshot snapshot, ResourceOwner owner)
+{
+	Snapshot		snap;
 
 	if (snapshot == InvalidSnapshot)
 		return InvalidSnapshot;
@@ -361,9 +375,9 @@ RegisterSnapshot(Snapshot snapshot)
 	snap = snapshot->copied ? snapshot : CopySnapshot(snapshot);
 
 	/* and tell resowner.c about it */
-	ResourceOwnerEnlargeSnapshots(CurrentResourceOwner);
+	ResourceOwnerEnlargeSnapshots(owner);
 	snap->regd_count++;
-	ResourceOwnerRememberSnapshot(CurrentResourceOwner, snap);
+	ResourceOwnerRememberSnapshot(owner, snap);
 
 	RegisteredSnapshots++;
 
@@ -383,10 +397,23 @@ UnregisterSnapshot(Snapshot snapshot)
 	if (snapshot == NULL)
 		return;
 
+	UnregisterSnapshotFromOwner(snapshot, CurrentResourceOwner);
+}
+
+/*
+ * UnregisterSnapshotFromOwner
+ * 		As above, but use the specified resource owner
+ */
+void
+UnregisterSnapshotFromOwner(Snapshot snapshot, ResourceOwner owner)
+{
+	if (snapshot == NULL)
+		return;
+
 	Assert(snapshot->regd_count > 0);
 	Assert(RegisteredSnapshots > 0);
 
-	ResourceOwnerForgetSnapshot(CurrentResourceOwner, snapshot);
+	ResourceOwnerForgetSnapshot(owner, snapshot);
 	RegisteredSnapshots--;
 	if (--snapshot->regd_count == 0 && snapshot->active_count == 0)
 	{
@@ -464,6 +491,26 @@ AtSubAbort_Snapshot(int level)
 }
 
 /*
+ * AtEarlyCommit_Snapshot
+ *
+ * Snapshot manager's cleanup function, to be called on commit, before
+ * doing resowner.c resource release.
+ */
+void
+AtEarlyCommit_Snapshot(void)
+{
+	/*
+	 * On a serializable transaction we must unregister our private refcount to
+	 * the serializable snapshot.
+	 */
+	if (registered_serializable)
+		UnregisterSnapshotFromOwner(CurrentSnapshot,
+									TopTransactionResourceOwner);
+	registered_serializable = false;
+
+}
+
+/*
  * AtEOXact_Snapshot
  * 		Snapshot manager's cleanup function for end of transaction
  */
@@ -475,13 +522,6 @@ AtEOXact_Snapshot(bool isCommit)
 	{
 		ActiveSnapshotElt	*active;
 
-		/*
-		 * On a serializable snapshot we must first unregister our private
-		 * refcount to the serializable snapshot.
-		 */
-		if (registered_serializable)
-			UnregisterSnapshot(CurrentSnapshot);
-
 		if (RegisteredSnapshots != 0)
 			elog(WARNING, "%d registered snapshots seem to remain after cleanup",
 				 RegisteredSnapshots);