MIME-Version: 1.0
References: <19438-9d37b179c56d43aa@postgresql.org> <1106026.1774573371@sss.pgh.pa.us>
 <1338824.1774633289@sss.pgh.pa.us> <CAApHDvox3Ro8mZJxignuyB-dGXJ9=wQNEkOFni9025GP=rOKkg@mail.gmail.com>
 <1830345.1774798374@sss.pgh.pa.us>
In-Reply-To: <1830345.1774798374@sss.pgh.pa.us>
From: David Rowley <dgrowleyml@gmail.com>
Date: Mon, 30 Mar 2026 12:41:37 +1300
Message-ID: <CAApHDvrdPriH6MO=YGEh9=KVDnDtqZyW4VuRXkmBS8WnRFessA@mail.gmail.com>
Subject: Re: BUG #19438: segfault with temp_file_limit inside cursor
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: kuzmin.db4@gmail.com, pgsql-bugs@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAApHDvrdPriH6MO%3DYGEh9%3DKVDnDtqZyW4VuRXkmBS8WnRFessA%40mail.gmail.com>
Precedence: bulk

On Mon, 30 Mar 2026 at 04:32, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Here's a fleshed-out version of the requested_size method.
> I noted that AllocSetRealloc needs a defense too, and then
> extended the patch to generation.c and slab.c.  bump.c
> doesn't have an issue, and I don't think alignedalloc.c
> needs its own defense either: it can rely on the underlying
> context type.

Thanks for writing that up.

I looked at the code and tested.  The only thing that I noted was
GenerationFree(), where we do:

/* Test for previously-freed chunk */
if (unlikely(chunk->requested_size == InvalidAllocSize))
    elog(WARNING, "detected double pfree in %s %p",
         ((MemoryContext) block->context)->name, chunk);
/* Test for someone scribbling on unused space in chunk */
Assert(chunk->requested_size < chunksize);

I expect you've likely thought of this, but if we do spit out the
warning there, then the Assert is definitely going to fail, as
InvalidAllocSize is defined as SIZE_MAX. I don't know if that means
it's worth deviating from the similar WARNINGs you've added and making
that one an ERROR. There's certainly no guarantee with the other
context that we'll not crash sometime very soon after issuing the
warning anyway, so maybe it's fine.

David