Re: Memory-leak in BackgroundWriter(and Checkpointer)

Stephen Frost <sfrost@snowman.net> writes:
> * Andres Freund (andres@2ndquadrant.com) wrote:
>> Seems more consistent with the rest of the code too. But anyway, I am
>> fine with fixing it either way.

> And this is really the other point- having LogStandbySnapshot() need to
> clean up after GetRunningTransactionLocks() but not
> GetRunningTransactionData() would strike me as very odd.

Meh.  I'm not impressed with permanently allocating an array large
enough to hold all the locks GetRunningTransactionLocks
might return --- that's potentially much larger than the other array,
and in fact I don't think we have a hard limit on its size at all.
Besides which, it's not like there is *no* cleanup for
GetRunningTransactionData --- it has a lock that has to be released ...

I think the proposed fix is fine code-wise; the real problem here is
crummy commenting.  GetRunningTransactionLocks isn't documented as
returning a palloc'd array, and why the heck do we have a long comment
about its implementation in LogStandbySnapshot?

            regards, tom lane