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

On 2026-04-07 Tu 5:10 AM, Jakub Wartak wrote:
> On Tue, Apr 7, 2026 at 5:55 AM jie wang<jugierwang(at)gmail(dot)com> wrote:
>>
>>
>> Andrew Dunstan<andrew(at)dunslane(dot)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