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

From: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
To: jie wang <jugierwang(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, 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-07 09:10:37
Message-ID: CAKZiRmyA0s-HF9=N5BZ4yJO_mE6_=uh0rNTwtXFve1vwQMH62A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2026-04-07 09:22:18 Re: Adding REPACK [concurrently]
Previous Message Alberto Piai 2026-04-07 09:09:44 Re: Adding a stored generated column without long-lived locks