Avoid crashing in GetOldestSnapshot() if there are no known snapshots.

tglsfdc · tglsfdc · commit 07a601eedab7 · 2016-08-07T14:36:02.000-04:00
The sole caller expects NULL to be returned in such a case, so make
it so and document it.

Per reports from Andreas Seltenreich and Regina Obe.  This doesn't
really fix their problem, as now their RETURNING queries will say
"ERROR: no known snapshots", but in any case this function should
not dump core in a reasonably-foreseeable situation.

Report: &lt;87vazemeda.fsf@credativ.de&gt;
Report: &lt;20160807051854.1427.32414@wrigleys.postgresql.org&gt;
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
@@ -399,14 +399,14 @@ GetLatestSnapshot(void)
 /*
  * GetOldestSnapshot
  *
- *		Get the oldest known snapshot, as judged by the LSN.
+ *		Get the transaction's oldest known snapshot, as judged by the LSN.
+ *		Will return NULL if there are no active or registered snapshots.
  */
 Snapshot
 GetOldestSnapshot(void)
 {
 	Snapshot	OldestRegisteredSnapshot = NULL;
 	XLogRecPtr	RegisteredLSN = InvalidXLogRecPtr;
-	XLogRecPtr	ActiveLSN = InvalidXLogRecPtr;
 
 	if (!pairingheap_is_empty(&RegisteredSnapshots))
 	{
@@ -416,10 +416,12 @@ GetOldestSnapshot(void)
 	}
 
 	if (OldestActiveSnapshot != NULL)
-		ActiveLSN = OldestActiveSnapshot->as_snap->lsn;
+	{
+		XLogRecPtr	ActiveLSN = OldestActiveSnapshot->as_snap->lsn;
 
-	if (XLogRecPtrIsInvalid(RegisteredLSN) || RegisteredLSN > ActiveLSN)
-		return OldestActiveSnapshot->as_snap;
+		if (XLogRecPtrIsInvalid(RegisteredLSN) || RegisteredLSN > ActiveLSN)
+			return OldestActiveSnapshot->as_snap;
+	}
 
 	return OldestRegisteredSnapshot;
 }

Original file line number	Diff line number	Diff line change
`@@ -399,14 +399,14 @@ GetLatestSnapshot(void)`
`399`	`399`	`/*`
`400`	`400`	`* GetOldestSnapshot`
`401`	`401`	`*`
`402`		`- * Get the oldest known snapshot, as judged by the LSN.`
	`402`	`+ * Get the transaction's oldest known snapshot, as judged by the LSN.`
	`403`	`+ * Will return NULL if there are no active or registered snapshots.`
`403`	`404`	`*/`
`404`	`405`	`Snapshot`
`405`	`406`	`GetOldestSnapshot(void)`
`406`	`407`	`{`
`407`	`408`	`Snapshot OldestRegisteredSnapshot = NULL;`
`408`	`409`	`XLogRecPtr RegisteredLSN = InvalidXLogRecPtr;`
`409`		`- XLogRecPtr ActiveLSN = InvalidXLogRecPtr;`
`410`	`410`
`411`	`411`	`if (!pairingheap_is_empty(&RegisteredSnapshots))`
`412`	`412`	`{`
`@@ -416,10 +416,12 @@ GetOldestSnapshot(void)`
`416`	`416`	`}`
`417`	`417`
`418`	`418`	`if (OldestActiveSnapshot != NULL)`
`419`		`- ActiveLSN = OldestActiveSnapshot->as_snap->lsn;`
	`419`	`+ {`
	`420`	`+ XLogRecPtr ActiveLSN = OldestActiveSnapshot->as_snap->lsn;`
`420`	`421`
`421`		`- if (XLogRecPtrIsInvalid(RegisteredLSN) \|\| RegisteredLSN > ActiveLSN)`
`422`		`- return OldestActiveSnapshot->as_snap;`
	`422`	`+ if (XLogRecPtrIsInvalid(RegisteredLSN) \|\| RegisteredLSN > ActiveLSN)`
	`423`	`+ return OldestActiveSnapshot->as_snap;`
	`424`	`+ }`
`423`	`425`
`424`	`426`	`return OldestRegisteredSnapshot;`
`425`	`427`	`}`