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

At Thu, 30 Jun 2022 12:28:43 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
> The root cause of these failures seems that sessionBackupState flag
> is not reset to SESSION_BACKUP_NONE even when BASE_BACKUP is aborted.
> So attached patch changes do_pg_abort_backup callback so that
> it resets sessionBackupState. I confirmed that, with the patch,
> those assertion failure and segmentation fault didn't happen.
>
> But this change has one issue that; if BASE_BACKUP is run while
> a backup is already in progress in the session by pg_backup_start()
> and that session is terminated, the change causes
> XLogCtl->Insert.runningBackups
> to be decremented incorrectly. That is, XLogCtl->Insert.runningBackups
> is incremented by two by pg_backup_start() and BASE_BACKUP,
> but it's decremented only by one by the termination of the session.
>
> To address this issue, I think that we should disallow BASE_BACKUP
> to run while a backup is already in progress in the *same* session
> as we already do this for pg_backup_start(). Thought? I included
> the code to disallow that in the attached patch.

It seems like to me that the root cause is the callback is registered
twice. The callback does not expect to be called more than once (at
least per one increment of runningBackups).

register_persistent_abort_backup_hanedler() prevents duplicate
regsitration of the callback so I think perform_base_backup should use
this function instead of protecting by the PG_*_ERROR_CLEANUP()
section.

Please find the attached.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center