Skip site navigation (1) Skip section navigation (2)

Re: SRF memory leaks

From: Neil Conway <neilc(at)samurai(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: SRF memory leaks
Date: 2008-02-26 08:01:54
Message-ID: 1204012914.12305.14.camel@goldbach (view raw or flat)
Thread:
Lists: pgsql-patches
On Mon, 2008-02-25 at 21:00 -0500, Tom Lane wrote:
> I find this part of the patch to be a seriously bad idea.
> nodeFunctionscan has no right to assume that the function has returned
> an expendable tupdesc; indeed, I would think that the other case is
> more nearly what's expected by the API for SRFs.

AFAICS this is not true of any of the SRFs in the backend, which always
return expendable tupdescs.

> A safer fix would be to try to make the tupdesc be in the new
> multi_call_ctx when it's being created by the funcapi.c code.

funcapi.c doesn't create the tupdesc, though: it is created by user
code, and merely assigned to a field of the RSI (note that the TupleDesc
in question is the one in the ReturnSetInfo, not in FuncCallContext.) A
minor detail is that the lifetime of the tupdesc also needs to exceed
the lifetime of the multi_call_ctx, at least as currently implemented.

>From looking at the existing SRFs in the backend, the TupleDesc is
always *explicitly* allocated in the estate's per-query context, so it
will leak unless freed. Perhaps it would be sufficient to FreeTupleDesc
iff tupdesc->refcount == -1?

BTW, I'm thinking of changing the various SRFs that make allocations in
the EState's per-query context to instead use the SRF's multi_call_ctx.
This means the memory will be reclaimed sooner and avoids possible
memory leaks.

-Neil



In response to

Responses

pgsql-patches by date

Next:From: Tom LaneDate: 2008-02-26 08:13:24
Subject: Re: SRF memory leaks
Previous:From: Tom LaneDate: 2008-02-26 02:03:36
Subject: Re: Shlib exports file refactoring

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group