Hi,
On 2026-09-22 14:54:33 +0000, Greg Burd wrote:
> BufferLockAcquire() only asserts that the current backend does not already
> hold
> a content lock on the buffer. I hit that assertion on a buildfarm animal, and
> while chasing it I convinced myself that the non-assert behaviour is worse
> than
> the crash in that it can permanently wedges the buffer.
That's completely normal for assertion paths IME.
> Patch attached to turn the assertion into an error. I am not proposing it
> as the whole fix; details and an open question below.
-0.5, it shows up in profiles for me. If we had support for something like
production assertions, with a lower overhead, I'd maybe +0.5 instead, but we
don't.
And I am fundamentally opposed to not make this an assertion failure. This is
a fundamentally broken program, that needs to trigger assertion
failures. There's way way too many contexts where an ERROR is basically
invisible.
> There is a bookkeeping asymmetry, PrivateRefCountData has room for a single
> lockmode...
>
> typedef struct PrivateRefCountData
> {
> int32 refcount; /* per-pin */
> BufferLockMode lockmode; /* per-buffer, ONE slot */
> } PrivateRefCountData;
>
> ... but the shared lock state is counted per acquisition:
There's no assymmetry, acquiring the same lock twice is fundamentally
wrong. It isn't supported. It was never supported. There's an assertion about
it.
> Also, error recovery does not rescue it.
Obviously not.
Greetings,
Andres Freund
