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.
