David Rowley <dgrowleyml@gmail.com> writes:
> 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.
Yeah, I saw that after sending the patch. Not only would that
Assert fail, but without it, code below would go nuts too.
> 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.
Seems like a reasonable answer. What do you think of making the
double-free cases ERRORs across the board? If we don't error out,
there will likely be cascading problems in all the mcxt types not
just this one.
regards, tom lane
