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

From Jakub Wartak
Subject Re: Add errdetail() with PID and UID about source of termination signal
Date
Msg-id CAKZiRmyA0s-HF9=N5BZ4yJO_mE6_=uh0rNTwtXFve1vwQMH62A@mail.gmail.com
Whole thread
In response to Re: Add errdetail() with PID and UID about source of termination signal  (jie wang <jugierwang@gmail.com>)
Responses Re: Add errdetail() with PID and UID about source of termination signal
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Alberto Piai
Date:
Subject: Re: Adding a stored generated column without long-lived locks
Next
From: Alvaro Herrera
Date:
Subject: Re: Adding REPACK [concurrently]