From: Chao Li <li.evan.chao@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Jim Jones <jim.jones@uni-muenster.de>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Jakub Wartak <jakub.wartak@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add errdetail() with PID and UID about source of termination signal
Date: Thu, 9 Apr 2026 11:11:08 +0800
Message-ID: <4D19BF33-3C16-49A3-918E-148369ADBCB2@gmail.com> (raw)
In-Reply-To: <jygesyr7mwg7ovdbxpmjvvbi3hccptpkcreqb645h7f56puwbz@hmkkwi3melfe>
References: <CFD6C011-7D95-418B-B7BA-5F864F682C83@gmail.com>
<CAKZiRmxvdYXhO1JrVLW3LkuoaTjYpKmhOzzOCOhw4wDM3rYRMQ@mail.gmail.com>
<1ba07c75-cc4e-410c-9af1-076feeeb9594@dunslane.net>
<cwyyryh2veejuxbj5ifzyaejw7jhhqc5mrdeq56xckknsdecn2@6hzfcxde2nm5>
<a076e20d-fc56-4b5d-aa47-a52b4e7a5061@dunslane.net>
<j7gm4f7bx7hprsc6j4tl47nd5r2vfv5aqevco76rsdawvkwfnd@nsbig7xqlqj5>
<9179378a-42db-4315-9ff6-b849ad5eefaf@dunslane.net>
<6cj4oxgdpavltoqdbydw2pgxfdbcejc26x3mitasrnswipxne5@7gkaeoibos4l>
<71930E58-82A8-4DDC-BA8C-5E394331E463@gmail.com>
<43ab327c-3ec7-4424-88de-22c7906e5529@uni-muenster.de>
<jygesyr7mwg7ovdbxpmjvvbi3hccptpkcreqb645h7f56puwbz@hmkkwi3melfe>
> On Apr 9, 2026, at 01:01, Andres Freund <andres@anarazel.de> wrote:
>
> 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
> <v1-0001-WIP-Support-for-extended-information-about-signal.patch>
I think the core idea here is to add a new parameter so signal handlers can receive “pg_signal_info". Compared to my earlier proposal, which stored sender info in file-scope variables, I agree this solution is more flexible. I think my earlier direction was mainly trying to avoid changing the signal handler interface.
So I have no objection to the overall direction. Since the patch is marked as WIP, I didn't review all the details yet. But one thing I want to point out is:
```
+typedef struct pg_signal_info
+{
+ pid_t pid; /* pid of sending process or 0 if unknown */
+ uid_t uid; /* uid of sending process or 0 if unknown */
+} pg_signal_info;
```
For uid, 0 is usually a valid value for root. So using 0 as the “unknown” value seems a bit awkward. Maybe we should instead document something like "uid is only meaningful when pid is not 0".
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: li.evan.chao@gmail.com, andres@anarazel.de, jim.jones@uni-muenster.de, andrew@dunslane.net, jakub.wartak@enterprisedb.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Add errdetail() with PID and UID about source of termination signal
In-Reply-To: <4D19BF33-3C16-49A3-918E-148369ADBCB2@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox