Detect double-release of spinlock

Hi,

On 2024-07-29 09:40:26 -0700, Andres Freund wrote:
> On 2024-07-29 12:33:13 -0400, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> > > On 2024-07-29 11:31:56 -0400, Tom Lane wrote:
> > >> There was some recent discussion about getting rid of
> > >> --disable-spinlocks on the grounds that nobody would use
> > >> hardware that lacked native spinlocks. But now I wonder
> > >> if there is a testing/debugging reason to keep it.
> >
> > > Seems it'd be a lot more straightforward to just add an assertion to the
> > > x86-64 spinlock implementation verifying that the spinlock isn't already free?
>
> FWIW, I quickly hacked that up, and it indeed quickly fails with 0393f542d72^
> and passes with 0393f542d72.

Thought it'd be valuable to post a patch to go along with this, to
-hackers. The thread started at [1]

Other context from this discussion:
> > I dunno, is that the only extra check that the --disable-spinlocks
> > implementation is providing?
>
> I think it also provides the (valuable!) check that spinlocks were actually
> initialized. But that again seems like something we'd be better off adding
> more general infrastructure for - nobody runs --disable-spinlocks locally, we
> shouldn't need to run this on the buildfarm to find problems like this.
>
>
> > I'm kind of allergic to putting Asserts into spinlocked code segments,
> > mostly on the grounds that it violates the straight-line-code precept.
> > I suppose it's not really that bad for tests that you don't expect
> > to fail, but still ...
>
> I don't think the spinlock implementation itself is really affected by that
> rule - after all, the --disable-spinlocks implementation actually consists out
> of several layers of external function calls (including syscalls in some
> cases!).

Greetings,

Andres Freund

[1] https://www.postgresql.org/message-id/E1sYSF2-001lEB-D1%40gemulon.postgresql.org