On Mon, Aug 29, 2016 at 11:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Aleksander Alekseev <a.alekseev@postgrespro.ru> writes:
>>> if (prodesc->user_proname == NULL || prodesc->internal_proname == NULL)
>>> + {
>>> + free(prodesc);
>
>> I think that prodesc->user_proname and prodesc->internal_proname should
>> also be freed if they are not NULL's.
>
> Hmm, this is kind of putting lipstick on a pig, isn't it? That code
> is still prone to leakage further down, because it calls stuff like
> SearchSysCache which is entirely capable of throwing elog(ERROR).
>
> If we're going to touch compile_pltcl_function at all, I'd vote for
>
> (1) changing these malloc calls to MemoryContextAlloc(TopMemoryContext,...
> (2) putting the cleanup into a PG_CATCH block, and removing all the
> retail free() calls that are there now.
We've done similar handling lately with for example 8c75ad43 for
plpython, and this has finished by using TopMemoryContext, so that's
the way to do. By the way plperl needs the same cleanup, and by
looking at the archives guess who did exactly that with a set of
patches not long ago:
https://www.postgresql.org/message-id/CAB7nPqRxvq+Q66UFzD9wa5UAftYN4WAUADbjXKFrync96kf-VQ@mail.gmail.com
But I did not get much feedback about those patches :)
So for now I have removed this part from the patch of this thread.
--
Michael
