Re: Fix a race condition in ConditionVariableTimedSleep()

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix a race condition in ConditionVariableTimedSleep()
Date: 2025-05-05 14:05:17
Message-ID: aBjFnZxujx22edMx@ip-10-97-1-34.eu-west-3.compute.internal
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

> But in first step, I doubt there should be any thing that cancels condition
> variable during WaitLatch.

I'm not 100% sure.

> Most probably you did wrong thing.

I "just" added an ereport(LOG,..) that has been enough to cancel the condition
variable and trigger the failed assertion. I agree that you could still consider
this as wrong thing if you think that we should not do anything that cancels
condition variable during WaitLatch though.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-05-05 14:09:26 Re: PG 18 release notes draft committed
Previous Message Alexander Pyhalov 2025-05-05 13:56:36 Re: MergeAppend could consider sorting cheapest child path