Postgres Pro
Downloads
Home   >   mailing lists

Thread: Fix memory leak when output postgres_fdw's "Relations"

Fix memory leak when output postgres_fdw's "Relations"

From
Ranier Vilela
Date:
Hi,


ExplainPropertyText does not save the *relations->data* pointer and
var relations goes out of scope.

No need to backpatch.

regards,
Ranier Vilela
Attachment

Re: Fix memory leak when output postgres_fdw's "Relations"

From
Tom Lane
Date:
Ranier Vilela <ranier.vf@gmail.com> writes:
> This is a minor leak, oversight in
>
https://github.com/postgres/postgres/commit/4526951d564a7eed512b4a0ac3b5893e0a115690#diff-e399f5c029192320f310a79f18c20fb18c8e916fee993237f6f82f05dad851c5

I don't think you understand how Postgres memory management works.
There's no permanent leak here, just till the end of the command;
so it's pretty doubtful that there's any need to expend cycles on
an explicit pfree.

            regards, tom lane

Re: Fix memory leak when output postgres_fdw's "Relations"

From
Ranier Vilela
Date:
Em sex., 23 de jul. de 2021 às 11:32, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> This is a minor leak, oversight in
> https://github.com/postgres/postgres/commit/4526951d564a7eed512b4a0ac3b5893e0a115690#diff-e399f5c029192320f310a79f18c20fb18c8e916fee993237f6f82f05dad851c5

I don't think you understand how Postgres memory management works.
Maybe not yet. Valgrind may also don't understand yet.
 
There's no permanent leak here, just till the end of the command;
so it's pretty doubtful that there's any need to expend cycles on
an explicit pfree.
Maybe.

==30691== 24 bytes in 1 blocks are definitely lost in loss record 123 of 469
==30691==    at 0x8991F0: MemoryContextAlloc (mcxt.c:893)
==30691==    by 0x899F29: MemoryContextStrdup (mcxt.c:1291)
==30691==    by 0x864E09: RelationInitIndexAccessInfo (relcache.c:1419)
==30691==    by 0x865F81: RelationBuildDesc (relcache.c:1175)
==30691==    by 0x868575: load_critical_index (relcache.c:4168)
==30691==    by 0x8684A0: RelationCacheInitializePhase3 (relcache.c:3980)
==30691==    by 0x88047A: InitPostgres (postinit.c:1031)
==30691==    by 0x773F12: PostgresMain (postgres.c:4081)
==30691==    by 0x6F9C33: BackendRun (postmaster.c:4506)
==30691==    by 0x6F96D8: BackendStartup (postmaster.c:4228)
==30691==    by 0x6F8C08: ServerLoop (postmaster.c:1745)
==30691==    by 0x6F747B: PostmasterMain (postmaster.c:1417)
 
Is this is a false-positive?

regards,
Ranier Vilela

Re: Fix memory leak when output postgres_fdw's "Relations"

From
Michael Paquier
Date:
On Fri, Jul 23, 2021 at 04:20:37PM -0300, Ranier Vilela wrote:
> Maybe not yet. Valgrind may also don't understand yet.

I think that you should do things the opposite way.  In short, instead
of attempting to understand first Valgrind or Coverity and then
Postgres, try to understand the internals of Postgres first and then
interpret what Valgrind or even Coverity tell you.

Tom is right.  There is no point in caring about the addition some
pfree()'s in the backend code as long as they don't prove to be an
actual leak in the context where a code path is used, and nobody will
follow you on that.  Some examples where this would be worth caring
about are things like tight loops leaking a bit of memory each time
these are taken, with leaks that can be easily triggered by the user
with some specific SQL commands, or even memory contexts not cleaned
up where they should, impacting some parts of the system (like the
executor, or even the planner) for a long-running analytical query.
--
Michael
Attachment