On 2023-03-01 14:47:51 -0800, Nathan Bossart wrote: > Subject: [PATCH v10 1/2] Move extra code out of the Pre/PostRestoreCommand() > block.
LGTM > From fb6957da01f11b75d1a1966f32b00e2e77c789a0 Mon Sep 17 00:00:00 2001 > From: Nathan Bossart <nathandboss...@gmail.com> > Date: Tue, 14 Feb 2023 09:44:53 -0800 > Subject: [PATCH v10 2/2] Don't proc_exit() in startup's SIGTERM handler if > forked by system(). > > Instead, emit a message to STDERR and _exit() in this case. This > change also adds assertions to proc_exit(), ProcKill(), and > AuxiliaryProcKill() to verify that these functions are not called > by a process forked by system(), etc. > --- > src/backend/postmaster/startup.c | 17 ++++++++++++++++- > src/backend/storage/ipc/ipc.c | 3 +++ > src/backend/storage/lmgr/proc.c | 2 ++ > src/backend/utils/error/elog.c | 28 ++++++++++++++++++++++++++++ > src/include/utils/elog.h | 6 +----- > 5 files changed, 50 insertions(+), 6 deletions(-) > diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c > index 22b4278610..b9e2c3aafe 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(MyProc->pid == (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(MyProc->pid == (int) getpid()); /* not safe if forked by > system(), etc. */ > > auxproc = &AuxiliaryProcs[proctype]; > I'd make these elog(PANIC), I think. The paths are not performance critical enough that a single branch hurts, so the overhead of the check is irrelevant, and the consequences of calling ProcKill() twice for the same process are very severe. > +/* > + * Write a message to STDERR using only async-signal-safe functions. This > can > + * be used to safely emit a message from a signal handler. > + * > + * TODO: It is likely possible to safely do a limited amount of string > + * interpolation (e.g., %s and %d), but that is not presently supported. > + */ > +void > +write_stderr_signal_safe(const char *fmt) As is, this isn't a format, so I'd probably just name it s or str :) > -/* > - * Write errors to stderr (or by equal means when stderr is > - * not available). Used before ereport/elog can be used > - * safely (memory context, GUC load etc) > - */ > extern void write_stderr(const char *fmt,...) pg_attribute_printf(1, 2); > +extern void write_stderr_signal_safe(const char *fmt); Not sure why you removed the comment? Greetings, Andres Freund
