Horton

Re: stopgap fix for signal handling during restore_command

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>, Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Fujii Masao <fujii@postgresql.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-25T19:07:42Z
Lists: pgsql-hackers 
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

Commits

  1. windows: msvc: Define STDIN/OUT/ERR_FILENO.

  2. Avoid calling proc_exit() in processes forked by system().

  3. Move extra code out of the Pre/PostRestoreCommand() section.