Re: Fix a race condition in ConditionVariableTimedSleep()

05.05.2025 17:05, Bertrand Drouvot пишет:
> Hi,
>
> On Mon, May 05, 2025 at 02:15:15PM +0300, Yura Sokolov wrote:
>> Interestingly, our colleague stepped into same problem recently [1] . It
>> happened because he attempted to make overcomplex timeout (SIGALARM) handler.
>>
>> But his solution was a bit different [2].
>>
>> [1] https://postgr.es/m/076eb7bd-52e6-4a51-ba00-c744d027b15c@postgrespro.ru
>> [2]
>> https://postgr.es/m/attachment/175030/0001-CV-correctly-handle-cv_sleep_target-change.patch
>>
>> And I believe, his solution is more elegant. Doesn't it?
>
> I'm not sure as it'd not maintain the initial intent to re-add to the
> wait list.

It will be re-added in next iteration of outer sleeping loop.

>> But in first step, I doubt there should be any thing that cancels condition
>> variable during WaitLatch.
>
> I'm not 100% sure.

There're no such things at the moment. WaitLatch doesn't check for
interrupts and doesn't do any other thing that could break
ConditionVariable. And I believe, if it start to do, we're going to be in
trouble.

> I "just" added an ereport(LOG,..) that has been enough to cancel the
condition
> variable and trigger the failed assertion.

It is because errfinish, called from ereport, invokes CHECK_FOR_INTERRUPTS.

Just wrap you ereport with HOLD_INTERRUPTS/RESUME_INTERRUPTS, and
everything will be "just" fine.

--
regards
Yura Sokolov aka funny-falcon