Re: Backup command and functions can cause assertion failure and segmentation fault

[Fujii Masao]

On 2022/07/01 15:09, Masahiko Sawada wrote:

The change looks good to me. I've also confirmed the change fixed the issues.

Thanks for the review and test!

@@ -233,6 +233,12 @@ perform_base_backup(basebackup_options *opt, bbsink *sink)
     StringInfo  labelfile;
     StringInfo  tblspc_map_file;
     backup_manifest_info manifest;
+   SessionBackupState status = get_backup_status();
+
+   if (status == SESSION_BACKUP_RUNNING)
+       ereport(ERROR,
+               (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                errmsg("a backup is already in progress in this session")));

I think we can move it to the beginning of SendBaseBackup() so we can
avoid bbsink initialization and cleanup in the error case.

Sounds good idea to me. I updated the patch in that way. Attached.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
index 8764084e21..c4d956b9c1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8783,6 +8783,8 @@ do_pg_abort_backup(int code, Datum arg)
        {
                XLogCtl->Insert.forcePageWrites = false;
        }
+
+       sessionBackupState = SESSION_BACKUP_NONE;
        WALInsertLockRelease();
 
        if (emit_warning)
diff --git a/src/backend/replication/basebackup.c 
b/src/backend/replication/basebackup.c
index 95440013c0..637c0ce459 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -949,6 +949,12 @@ SendBaseBackup(BaseBackupCmd *cmd)
 {
        basebackup_options opt;
        bbsink     *sink;
+       SessionBackupState status = get_backup_status();
+
+       if (status == SESSION_BACKUP_RUNNING)
+               ereport(ERROR,
+                               
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                errmsg("a backup is already in progress in 
this session")));
 
        parse_basebackup_options(cmd->options, &opt);