Re: pgsql: Fix double-release of spinlock

Andres Freund <andres@anarazel.de> writes:
> On 2024-07-29 12:45:19 -0400, Tom Lane wrote:
>> Hmm, but how?

> ...
> I.e. the version using 2 as the locked state uses a three byte instruction vs
> a two byte instruction before.
> *If* we are worried about this, we could
> a) Change the representation only for assert enabled builds, but that'd have
>    ABI issues again.

Agreed, that would be a very bad idea.  It would for example break the
case of a non-assert-enabled extension used with an assert-enabled
core or vice versa, which is something we've gone out of our way to
allow.

> b) Instead define the spinlock to have 1 as the unlocked state and 0 as the
>    locked state. That makes it a bit harder to understand that initialization
>    is missing, compared to a dedicated state, as the first use of the spinlock
>    just blocks.

This option works for me.

> To make 1) b) easier to understand it might be worth changing the slock_t
> typedef to be something like

> typedef struct slock_t
> {
>         char is_free;
> } slock_t;

+1

How much of this would we change across platforms, and how much
would be x86-only?  I think there are enough people developing on
ARM (e.g. Mac) now to make it worth covering that, but maybe we
don't care so much about anything else.

            regards, tom lane