"Andres Freund" <and...@anarazel.de> writes:
> On Tue, Mar 16, 2021, at 20:01, Tom Lane wrote:
>> That seems both unnecessary and impractical. We have to consider that
>> everything-still-reachable is an OK final state.
> I don't consider "still reachable" a leak. Just definitely unreachable.
OK, we're in violent agreement then -- I must've misread what you wrote.
>>> I think the run might have shown a genuine leak:
>> I didn't see anything like that after applying the fixes I showed before.
> I think it's actually unreachable memory (unless you count resetting the
> cache context), based on manually tracing the code... I'll try to repro.
I agree that having to reset CacheContext is not something we
should count as "still reachable", and I'm pretty sure that the
existing valgrind infrastructure doesn't count it that way.
As for the particular point about ParallelBlockTableScanWorkerData,
I agree with your question to David about why that's in TableScanDesc
not HeapScanDesc, but I can't get excited about it not being freed in
heap_endscan. That's mainly because I do not believe that anything as
complex as a heap or indexscan should be counted on to be zero-leakage.
The right answer is to not do such operations in long-lived contexts.
So if we're running such a thing while switched into CacheContext,
*that* is the bug, not that heap_endscan didn't free this particular
allocation.
regards, tom lane
