Hi,
On 2023-02-23 20:33:23 -0800, Nathan Bossart wrote:>
> if (in_restore_command)
> - proc_exit(1);
> + {
> + /*
> + * If we are in a child process (e.g., forked by system() in
> + * RestoreArchivedFile()), we don't want to call any exit
> callbacks.
> + * The parent will take care of that.
> + */
> + if (MyProcPid == (int) getpid())
> + proc_exit(1);
> + else
> + {
> + const char msg[] = "StartupProcShutdownHandler()
> called in child process\n";
> + int rc pg_attribute_unused();
> +
> + rc = write(STDERR_FILENO, msg, sizeof(msg));
> + _exit(1);
> + }
> + }
Why do we need that rc variable? Don't we normally get away with (void)
write(...)?
> diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
> index 22b4278610..e3da0622d7 100644
> --- a/src/backend/storage/lmgr/proc.c
> +++ b/src/backend/storage/lmgr/proc.c
> @@ -805,6 +805,7 @@ ProcKill(int code, Datum arg)
> dlist_head *procgloballist;
>
> Assert(MyProc != NULL);
> + Assert(MyProcPid == (int) getpid()); /* not safe if forked by
> system(), etc. */
>
> /* Make sure we're out of the sync rep lists */
> SyncRepCleanupAtProcExit();
> @@ -925,6 +926,7 @@ AuxiliaryProcKill(int code, Datum arg)
> PGPROC *proc;
>
> Assert(proctype >= 0 && proctype < NUM_AUXILIARY_PROCS);
> + Assert(MyProcPid == (int) getpid()); /* not safe if forked by
> system(), etc. */
>
> auxproc = &AuxiliaryProcs[proctype];
>
> --
> 2.25.1
I think the much more interesting assertion here would be to check that
MyProc->pid equals the current pid.
Greetings,
Andres Freund
