Re: Buffer locking is special (hints, checksums, AIO writes)

Hi,

On 2026-01-29 12:27:10 -0500, Andres Freund wrote:
> In the crashes due to Alexander's repro that I have looked at, the page is
> returned by SpGistNewBuffer()->GetFreeIndexPage(). Which afaict should only
> contain empty pages. It's easy to see how that could happen with concurrency,
> but there's none in the test.

I think I see how that happens - we reenter the page into the FSM during the
VACUUM that's part of the repro, but we previously also "recorded" it with
SpGistSetLastUsedPage(), presumably before it was emptied during vacuum. The
we reuse the page first via the "last used page" cache, making it not
empty. Then we get the same page via the FSM, which wasn't updated when we
started filling the page via the last-used-page mechanism.

> I was just trying to repro this again while writing this message, and
> interestingly I got the same issue in nbtree this time. Which a) confirms
> Peter's statement that the "conditionally locking a buffer we already locked"
> issue exists for nbtree b) makes me suspect something odd is happening around
> indexfsm.

Not sure how that happens in a single threaded workload yet, though.

> diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
> index 6f935648ae9..f5602f4e7e1 100644
> --- a/src/backend/storage/buffer/bufmgr.c
> +++ b/src/backend/storage/buffer/bufmgr.c
> @@ -5895,6 +5895,13 @@ BufferLockUnlock(Buffer buffer, BufferDesc *buf_hdr)
>
> /*
> * Acquire the content lock for the buffer, but only if we don't have to wait.
> + *
> + * It is allowed to try to conditionally acquire a lock on a buffer that this
> + * backend has already locked, but the lock acquisition will always fail, even
> + * if the new lock acquisition does not conflict with an already held lock
> + * (e.g. two share locks). This is because we don't track per-backend
> + * ownership of multiple lock levels. That is ok for the current uses of
> + * BufferLockConditional().
> */
> static bool
> BufferLockConditional(Buffer buffer, BufferDesc *buf_hdr, BufferLockMode mode)
> @@ -5902,11 +5909,16 @@ BufferLockConditional(Buffer buffer, BufferDesc *buf_hdr, BufferLockMode mode)
> PrivateRefCountEntry *entry = GetPrivateRefCountEntry(buffer, true);
> bool mustwait;
>
> - /*
> - * We better not already hold a lock on the buffer.
> - */
> Assert(entry->data.lockmode == BUFFER_LOCK_UNLOCK);
>

Err, this should have been removed, I accidentally re-added the hunk while
experimenting.

Greetings,

Andres Freund