Re: Yes, WaitLatch is vulnerable to weak-memory-ordering bugs

On 08.08.2011 19:39, Tom Lane wrote:
> Robert Haas<robertmhaas(at)gmail(dot)com> writes:
>> On the flip side, I'm not sure that anyone ever really expected that a
>> latch could be safely used this way. Normally, I'd expect the flag to
>> be some piece of state protected by an LWLock, and I think that ought
>> to be OK provided that the lwlock is released before setting or
>> checking the latch. Maybe we should try to document the contract here
>> a little better; I think it's just that there must be a full memory
>> barrier (such as LWLockRelease) in both processes involved in the
>> interaction.
>
> Heikki argued the same thing in
> http://archives.postgresql.org/message-id/4CEA5A0F.1030602@enterprisedb.com
> but I don't think anyone has actually done the legwork to verify that
> current uses are safe. So [ ... warms up grep ... ]

Thanks for double-checking.

> (3) I'm not going to hold still for not inserting a memory barrier
> into SetLatch, once we have the code. Unsubstantiated performance
> worries are not a sufficient argument not to --- as a wise man once
> said, you can make the code arbitrarily fast if it doesn't have to
> give the right answer.

I agree we definitely should add the memory barrier instruction there
once we have them.

> (4) In the meantime, we'd better document that it's caller's
> responsibility to ensure that the flag variable is adequately
> protected by locking. I think SyncRepWaitForLSN could use a warning
> comment, too. I will go do that.

Ok, thanks.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com