Re: Add errdetail() with PID and UID about source of termination signal - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: Add errdetail() with PID and UID about source of termination signal
Date
Msg-id f38b3af1-dd57-4e24-80c6-21a680f003a9@dunslane.net
Whole thread Raw
In response to Re: Add errdetail() with PID and UID about source of termination signal  (Jakub Wartak <jakub.wartak@enterprisedb.com>)
List pgsql-hackers


On 2026-04-07 Tu 5:10 AM, Jakub Wartak wrote:
On Tue, Apr 7, 2026 at 5:55 AM jie wang <jugierwang@gmail.com> wrote:


Andrew Dunstan <andrew@dunslane.net> 于2026年4月7日周二 00:51写道:

[..]
I'd kinda like to sneak this in for pg19, because I think it's useful.
Here's a v6 that changes one or two things:


- changes the globals to sig_atomic_t

- in ProcessInterrupts, copies to local sender_pid/sender_uid, then
zeros the globals before any ereport

- uses errdetail() for all the messages


Plus a few more cosmetic changes like consistent casing.


cheers


andrew



--
Andrew Dunstan
EDB: https://www.enterprisedb.com


I just reviewed v6 and got 1 comment.

sig_atomic_t is underlying int, but pid_t and uid_t are usually unsigned int,
so that while saving pid and uid to ProcDieSenderPid and ProcDieSenderUid,
overflow may happen. But that’s fine, as the data is stored in the same way
and we only want to print them. So, for the print statement:

#define ERRDETAIL_SIGNAL_SENDER(pid, uid) \    ((pid) == 0 ? 0 : \    errdetail("Signal sent by PID %d, UID %d.", (int) (pid), (int) (uid)))

Does it make sense to use %u and cast to pid_t and uid_t? Like:

#define ERRDETAIL_SIGNAL_SENDER(pid, uid) \    ((pid) == 0 ? 0 : \    errdetail("Signal sent by PID %u, UID %u.", (pid_t) (pid), (uid_t) (uid)))
I really don't have hard opinion on what we should use here (int vs uint32 vs
pid_t)  and I was scratching my head at this earier, as:

1. Usually win32 doesn't have pid_t, but in win32_port.h we have   "typedef int pid_t".

2. According to `grep -ri ' pid' src/backend/ we seem to use "int" in most cases   for this (especially MyProcPid is also int). The only other places
where I found   %u or %l would be those:
   src/backend/port/win32/signal.c:        snprintf(pipename, sizeof(pipename),
"\\\\.\\pipe\\pgsignal_%u", (int) pid);
   src/backend/port/win32/signal.c:        (errmsg("could not create signal listener pipe for PID %d:
error code %lu",   src/backend/postmaster/datachecksum_state.c:        "Waiting for worker in database %s (pid %ld)", db->dbname, (long) pid);   src/backend/postmaster/postmaster.c:        elog(DEBUG3, "kill(%ld,%d) failed: %m", (long) pid, signal);
   So apparently we are really not consistent, but int looks fine to me.

3. Linux kernel for most allows up to kernel.pid_max (4194304, 22 bits) and   internally it uses "int" for it's pid_max_max [1] and uses up to
30-bits since always.

So "int" follows old style and seems to be OK at least from my point of view.

-J.

[1] - https://github.com/torvalds/linux/blob/master/kernel/pid.c#L64C17-L64C40
[2] - https://github.com/torvalds/linux/blob/master/include/linux/threads.h#L34



OK, went back to using int. Pushed.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: enable fallthrough warnings on clang
Next
From: Heikki Linnakangas
Date:
Subject: Re: Assertion failure in hash_kill_items()