| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add errdetail() with PID and UID about source of termination signal |
| Date: | 2026-04-08 09:13:40 |
| Message-ID: | 43ab327c-3ec7-4424-88de-22c7906e5529@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi
On 08/04/2026 03:03, Chao Li wrote:
> I tried to understand the layering comment, and I’m proposing a fix where sender information is stored within pqsignal and exposed via a new helper function, pqsignal_get_sender(). Then the signal handler retrieves the signal sender via pqsignal_get_sender() and sets ProcDieSenderPid/ProcDieSenderUid. Please see the attached diff.
I have a few questions (slightly unrelated to Chao's fix)
I'm a bit confused with the data flow and the data types involved:
1) in pgsignal.c it's volatile sig_atomic_t
#if !defined(FRONTEND) && defined(HAVE_SA_SIGINFO)
static volatile sig_atomic_t pqsignal_has_sender[PG_NSIG];
static volatile sig_atomic_t pqsignal_sender_pid[PG_NSIG];
static volatile sig_atomic_t pqsignal_sender_uid[PG_NSIG];
#endif
2) in postgres.c it's int
int sender_pid;
int sender_uid;
3) and in globals.c it is volatile int
volatile int ProcDieSenderPid = 0;
volatile int ProcDieSenderUid = 0;
I guess 2) is ok, since it seems to be a one time read, but I'm
wondering if the consumer in 3) should also use volatile sig_atomic_t:
in globals.c
volatile sig_atomic_t ProcDieSenderPid = 0;
volatile sig_atomic_t ProcDieSenderUid = 0;
in miscadmin.h
extern PGDLLIMPORT volatile sig_atomic_t ProcDieSenderPid;
extern PGDLLIMPORT volatile sig_atomic_t ProcDieSenderUid;
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
.. 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? If this is the case, the change can be quite
substantial.
Best, Jim
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Soumya S Murali | 2026-04-08 09:17:28 | Re: Fix bug with accessing to temporary tables of other sessions |
| Previous Message | Chao Li | 2026-04-08 09:08:36 | Use proc_exit() in WalRcvWaitForStartPosition |