Re: Add errdetail() with PID and UID about source of termination signal

Hi,

Attached is a very rough first draft for how I think this needs to look like.

Basically, SIGNAL_INFO always will pass both the signal number and extended
information along to the signal handler. The extended information is a
postgres specific struct. If the platform can't provide the extended
information, the values are instead set to some default value indicating that
the information is not known.

With that die() (and also StatementCancelHandler, ...) can just set whatever
globals it wants, without pqsignal.c needing to know about it.

It also allows us to extend the amount of information in the future. E.g. I'd
like to log the reason for a segfault (could e.g. be an OOM kill or an umapped
region) to stderr.

The annoying thing about it is needing to change nearly all the existing
references to SIG_IGN/SIG_DFL, to avoid warnings due to mismatched types.


On 2026-04-08 11:13:40 +0200, Jim Jones wrote:
> If I understood this thread correctly, the feature (v6) introduced a
> problematic dual signature for wrapper_handler in pgsignal.c:

> +#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO)
> +static void
> +wrapper_handler(int signo, siginfo_t * info, void *context)
> +#else
>  static void
>  wrapper_handler(SIGNAL_ARGS)
> +#endif

I think it's not a problem for wrapper_handler to change its signature, that's
a local implementation detail.  The problem is that the way the arguments were
passed was just wrong.  Because signal handlers can be nested and such
nastiness, doing any of that via global variables is a recipe for disaster.
It's also just ugly.


> .. and it should be rather done in the source (c.h) where SIGNAL_ARGS is
> defined:
> 
> #ifndef SIGNAL_ARGS
> #define SIGNAL_ARGS  int postgres_signal_arg
> #endif
> 
> Something like:
> 
> #ifndef SIGNAL_ARGS
> #ifdef HAVE_SA_SIGINFO
> #define SIGNAL_ARGS  int postgres_signal_arg, siginfo_t
> *postgres_signal_info, void *postgres_signal_context
> #else
> #define SIGNAL_ARGS  int postgres_signal_arg
> #endif
> #endif
> 
> But wouldn't it mean that all handlers need to be updated as well, since
> they'd get new parameters?

All the signal handlers actually use SIGNAL_ARGS themselves, so that part is
not a problem.

However, if we did it like you sketch above, they'd all need ifdefs etc to be
able to access any extended information, which seems like a terrible
idea. Especially if we want this information on multiple platforms, where the
struct to be passed would differ.

Hence in my prototype it's hidden behind a platform indepenedent struct of our
own.


> If this is the case, the change can be quite substantial.

It's a bit annoying to do all the s/\b(SIG_(IGN|DFL)/PG_$1/, but it's not that
bad, I think?

I unfortunately don't see a good other way to deal with it.  We could have a
macro wrapper around pqsignal() that checks for SIG_IGN with a cast to the
system type, but that seems exceedingly ugly.

Greetings,

Andres Freund