Re: Usage of ProcessConfigfile in SIGHUP_Handler - Mailing list pgsql-hackers

From Lakshmi Narayana Velayudam
Subject Re: Usage of ProcessConfigfile in SIGHUP_Handler
Date
Msg-id CAA4pTnLojjsMczEW0ORBV2C_E0NNGSjPADngxxmFm4dy20ekqw@mail.gmail.com
Whole thread Raw
In response to Re: Usage of ProcessConfigfile in SIGHUP_Handler  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Usage of ProcessConfigfile in SIGHUP_Handler
List pgsql-hackers
On Thu, Aug 22, 2024 at 9:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> The previous postmaster coding blocked signals
> everywhere except immediately around the main loop's select() call,
> so there wasn't any real hazard of signal handlers interrupting
> anything of concern.  We redid it for cleanliness, not because there
> was any observable bug.


Agreed, signal handlers are very sensitive (at least as of this moment) and the current approach looks very clean and safe.

> No, it was not.
> (If there had been a bug there, ProcessConfigFile would have been
> the least of our problems, because all the other postmaster signals
> were handled in the same style.)

Just as an info for future readers, it is indeed a bug for two reasons
1. changin GUC values in the signal handler, compiler might optimise the values so main control wouldn't have know about it later (90% sure about this)
2. ProcessConfigFile calls AllocSetCreate which in turn calls malloc which is not async signal safe(see man 7 signal-safety) and can cause deadlock in certain situations. (Dig malloc internal code if interested) 
Not only in SIGHUP handler it is done in other handlers as well. Feel free to correct me if there is any inaccuracy in what I said.

Regards,
Narayana

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Next
From: Alexander Korotkov
Date:
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands