1 files changed, 35 insertions, 46 deletions

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 27085b15a83..df50c2af7a7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -679,8 +679,6 @@ static void ReadControlFile(void);
 static void UpdateControlFile(void);
 static char *str_time(pg_time_t tnow);
 
-static void pg_backup_start_callback(int code, Datum arg);
-
 static int	get_sync_bit(int method);
 
 static void CopyXLogRecordToWAL(int write_len, bool isLogSwitch,
@@ -8271,7 +8269,7 @@ void
 do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 				   BackupState *state, StringInfo tblspcmapfile)
 {
-	bool		backup_started_in_recovery = false;
+	bool		backup_started_in_recovery;
 
 	Assert(state != NULL);
 	backup_started_in_recovery = RecoveryInProgress();
@@ -8320,8 +8318,12 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 	XLogCtl->Insert.forcePageWrites = true;
 	WALInsertLockRelease();
 
-	/* Ensure we release forcePageWrites if fail below */
-	PG_ENSURE_ERROR_CLEANUP(pg_backup_start_callback, (Datum) 0);
+	/*
+	 * Ensure we release forcePageWrites if fail below. NB -- for this to work
+	 * correctly, it is critical that sessionBackupState is only updated after
+	 * this block is over.
+	 */
+	PG_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, DatumGetBool(true));
 	{
 		bool		gotUniqueStartpoint = false;
 		DIR		   *tblspcdir;
@@ -8531,7 +8533,7 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 
 		state->starttime = (pg_time_t) time(NULL);
 	}
-	PG_END_ENSURE_ERROR_CLEANUP(pg_backup_start_callback, (Datum) 0);
+	PG_END_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, DatumGetBool(true));
 
 	state->started_in_recovery = backup_started_in_recovery;
 
@@ -8541,23 +8543,6 @@ do_pg_backup_start(const char *backupidstr, bool fast, List **tablespaces,
 	sessionBackupState = SESSION_BACKUP_RUNNING;
 }
 
-/* Error cleanup callback for pg_backup_start */
-static void
-pg_backup_start_callback(int code, Datum arg)
-{
-	/* Update backup counters and forcePageWrites on failure */
-	WALInsertLockAcquireExclusive();
-
-	Assert(XLogCtl->Insert.runningBackups > 0);
-	XLogCtl->Insert.runningBackups--;
-
-	if (XLogCtl->Insert.runningBackups == 0)
-	{
-		XLogCtl->Insert.forcePageWrites = false;
-	}
-	WALInsertLockRelease();
-}
-
 /*
  * Utility routine to fetch the session-level status of a backup running.
  */
@@ -8850,38 +8835,42 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
  * system out of backup mode, thus making it a lot more safe to call from
  * an error handler.
  *
- * The caller can pass 'arg' as 'true' or 'false' to control whether a warning
- * is emitted.
+ * 'arg' indicates that it's being called during backup setup; so
+ * sessionBackupState has not been modified yet, but runningBackups has
+ * already been incremented.  When it's false, then it's invoked as a
+ * before_shmem_exit handler, and therefore we must not change state
+ * unless sessionBackupState indicates that a backup is actually running.
  *
- * NB: This gets used as a before_shmem_exit handler, hence the odd-looking
- * signature.
+ * NB: This gets used as a PG_ENSURE_ERROR_CLEANUP callback and
+ * before_shmem_exit handler, hence the odd-looking signature.
  */
 void
 do_pg_abort_backup(int code, Datum arg)
 {
-	bool		emit_warning = DatumGetBool(arg);
+	bool		during_backup_start = DatumGetBool(arg);
 
-	/*
-	 * Quick exit if session does not have a running backup.
-	 */
-	if (sessionBackupState != SESSION_BACKUP_RUNNING)
-		return;
+	/* Only one of these conditions can be true */
+	Assert(during_backup_start ^
+		   (sessionBackupState == SESSION_BACKUP_RUNNING));
 
-	WALInsertLockAcquireExclusive();
-	Assert(XLogCtl->Insert.runningBackups > 0);
-	XLogCtl->Insert.runningBackups--;
-
-	if (XLogCtl->Insert.runningBackups == 0)
+	if (during_backup_start || sessionBackupState != SESSION_BACKUP_NONE)
 	{
-		XLogCtl->Insert.forcePageWrites = false;
-	}
+		WALInsertLockAcquireExclusive();
+		Assert(XLogCtl->Insert.runningBackups > 0);
+		XLogCtl->Insert.runningBackups--;
 
-	sessionBackupState = SESSION_BACKUP_NONE;
-	WALInsertLockRelease();
+		if (XLogCtl->Insert.runningBackups == 0)
+		{
+			XLogCtl->Insert.forcePageWrites = false;
+		}
+
+		sessionBackupState = SESSION_BACKUP_NONE;
+		WALInsertLockRelease();
 
-	if (emit_warning)
-		ereport(WARNING,
-				(errmsg("aborting backup due to backend exiting before pg_backup_stop was called")));
+		if (!during_backup_start)
+			ereport(WARNING,
+					errmsg("aborting backup due to backend exiting before pg_backup_stop was called"));
+	}
 }
 
 /*
@@ -8895,7 +8884,7 @@ register_persistent_abort_backup_handler(void)
 
 	if (already_done)
 		return;
-	before_shmem_exit(do_pg_abort_backup, DatumGetBool(true));
+	before_shmem_exit(do_pg_abort_backup, DatumGetBool(false));
 	already_done = true;
 }