Re: GetBufferDescriptor() being called for local buffers from MarkBufferDirtyHint()

Hi,

On Wed, 1 Jul 2026 at 11:04, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
wrote:

> On Wed, Jul 1, 2026 at 12:47 AM Ayush Tiwari
> <ayushtiwari.slg01@gmail.com> wrote:
> > It looks like InitLocalBuffers() initializes the descriptors in a loop
> before
> > it sets NLocBuffer, so during that loop the assert sees NLocBuffer ==
> 0.  The
> > access itself seems fine, since the array is already allocated with
> > num_temp_buffers entries; it's just that this is the one place that runs
> > before the bound the assert checks against is set.  (The shared path
> doesn't
> > run into this, since NBuffers is already set when
> BufferManagerShmemInit()
> > does the equivalent loop.)
> >
> > I'm not sure which way would be preferable here if are adding assert for
> > LocalBufferDescriptor too:
> >
> >   1. set NLocBuffer = nbufs before the init loop, so the loop can keep
> using
> >      GetLocalBufferDescriptor(); or
>
> This looks safe for me. The callers of InitLocalBuffers rely on
> LocalBufferHash to decide whether or not local buffers have been
> initialized. Further local buffers are local to the backend,
> concurrent access is not possible. So, even if we initialize
> NLocalBuffer before actually allocating the buffers, nobody is going
> to access the local buffers before they are fully initialized.
> LocalBufferHash still serves as a definitive condition to decide
> whether or not local buffers are initialized.
>

Thanks for looking! This matches my analysis as well.

>
> >   2. leave NLocBuffer where it is and index LocalBufferDescriptors[]
> directly
> >      in that loop.
>
> I had suggested this in the context of shared buffers and it was shot
> down by Michael. So probably the same argument applies here.
>

Ahh I see, good to know it was already considered.

If 1 is not possible for some reason, you could augment your assertion
> as Assert(id >= 0 && (!LocalHashBuffers || id < NLocBuffer)) or
> separate the two operands of && into separate assertions.
>

I'm attaching a patch with option 1, ideally I would not want to
extend the assertion, but if the option is not feasible for some reason,
we'll have to go this way.

v2 attached sets NLocBuffer = nbufs before the initialization loop, so the
loop can keep using GetLocalBufferDescriptor(), and drops the now-redundant
assignment at the end.

Regards,
Ayush