Hi, On Wed, 26 Aug 2026 at 17:53, Heikki Linnakangas <[email protected]> wrote:
> On 26/08/2026 08:59, Ayush Tiwari wrote: > > This approach looks good to me overall. I built the v2 series with > > assertions and injection points enabled, and all five tests passed. > > > > I had one question about the lifetime of the options copy. It is still > > allocated in TopMemoryContext, while the after-startup path no longer > calls > > pfree(request->options). The PG_FINALLY block clears > > pending_shmem_requests, but could that leave the options allocated until > > backend exit? The impact seems small, but perhaps the options should be > > freed before clearing the list, or allocated in the same context as the > > requests? > > You're right. I changed it to use TopMemoryContext like you had it > originally. That seems more clear, after all. > > > One minor test nit: > > > > - The comment says "A failure in the requesting shared memory", but the > > injection point triggers in test_shmem_init(), so would "initializing > > shared memory" be more accurate? > > Yeah. I did some other cleanups in the test too, and merged the tests > into the commits with the code fixes. Here's a new version, if you want > to have one final look. > I took a quick look and triggered the tests parallely. Patch looks good to me. Regards, Ayush
