Re: pgsql: Get rid of the dedicated latch for signaling the startup process

On 2020/11/05 5:36, Heikki Linnakangas wrote:
> On 04/11/2020 15:17, Heikki Linnakangas wrote:
>> On 04/11/2020 14:03, Fujii Masao wrote:
>>> Or ISTM that WakeupRecovery() should set the latch only when the latch
>>> has not been reset to NULL yet.
>>
>> Got to be careful with race conditions, if the latch is set to NULL at
>> the same time that WakeupRecovery() is called.
>
> I don't think commit 113d3591b8 got this quite right:
>
>> void
>> WakeupRecovery(void)
>> {
>>     if (XLogCtl->recoveryWakeupLatch)
>>         SetLatch(XLogCtl->recoveryWakeupLatch);
>> }
>
> If XLogCtl->recoveryWakeupLatch is set to NULL between the if and the SetLatch, you'll still get a segfault. That's highly unlikely to happen in practice because the compiler will optimize that into a single load instruction, but could happen with -O0. I think you'd need to do the access only once, using a volatile pointer, to make that safe. Maybe it's simpler to just not reset it to NULL, after all.

Yes, you're right. I agree it's simpler to remove the code resetting
the latch to NULL. Also as the comment for that code explains,
basically it's not necessary to reset it to NULL.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION