Re: Use generation memory context for tuplestore.c

On Sat, 6 Jul 2024 at 02:00, Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
> CREATE TABLE t(f int, t int);
> INSERT INTO t VALUES (1, 1);
>
> WITH RECURSIVE sg(f, t) AS (
> SELECT * FROM t t1
> UNION ALL
> SELECT t2.* FROM t t2, sg WHERE t2.f = sg.t
> ) SEARCH DEPTH FIRST BY f, t SET seq
> SELECT * FROM sg;
> " | timeout 60 psql
>
> triggers
> TRAP: failed Assert("chunk->requested_size < oldsize"), File: "generation.c", Line: 842, PID: 830294

This seems to be a bug in GenerationRealloc().

What's happening is when we palloc(4) for the files array in
makeBufFile(), that palloc uses GenerationAlloc() and since we have
MEMORY_CONTEXT_CHECKING, the code does:

/* ensure there's always space for the sentinel byte */
chunk_size = MAXALIGN(size + 1);

resulting in chunk_size == 8. When extendBufFile() effectively does
the repalloc(file->files, 8), we call GenerationRealloc() with those 8
bytes and go into the "if (oldsize >= size)" path thinking we have
enough space already. Here both values are 8, which would be fine on
non-MEMORY_CONTEXT_CHECKING builds, but there's no space for the
sentinel byte here. set_sentinel(pointer, size) stomps on some memory,
but no crash from that. It's only a problem when extendBufFile() asks
for 12 bytes that we come back into GenerationRealloc() and trigger
the Assert(chunk->requested_size < oldsize). Both of these values are
8. The oldsize should have been large enough to store the sentinel
byte, it isn't due to the problem caused during GenerationRealloc with
8 bytes.

I also had not intended that the buffile.c stuff would use the
generation context. I'll need to fix that too, but I think I'll fix
the GenerationRealloc() first.

The attached fixes the issue. I'll stare at it a bit more and try to
decide if that's the best way to fix it.

David