Re: Facing a problem with SPI

From: "Gurjeet Singh" <singh(dot)gurjeet(at)gmail(dot)com>
To: "PGSQL Hackers" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Facing a problem with SPI
Date: 2006-12-03 12:25:16
Message-ID: 65937bea0612030425t34c3d722m9dea723e32b8d96c@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Just noticed that this was committed by Tom on 21/Nov with this log
message:

<snip>
Prevent intratransaction memory leak when a subtransaction is aborted
in the middle of executing a SPI query. This doesn't entirely fix the
problem of memory leakage in plpgsql exception handling, but it should
get rid of the lion's share of leakage.
</snip>

Can the memory leaks be avoided in some other way?

Best regards,

--
gurjeet[(dot)singh](at)EnterpriseDB(dot)com
singh(dot)gurjeet(at){ gmail | hotmail | yahoo }.com

On 12/3/06, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> wrote:
>
> Hi all,
>
> I am facing a problem with the SPI (spi.c). I have implemented
> short-lived virtual indexes using SubTransactions. The Index Adviser, when
> creating virtual indexes, creates them inside a SubTransaction, and when it
> is done with its advisory stuff, it rolls back the SubTransaction. This
> achieves two objectives:
>
> (1) Easy cleanup of dependencies/changes in the catalog.
> (2) The virtual indexes are not visible to other backends. This
> eliminates the possibility of other backends (not running in Advise mode)
> seeing these indexes, as these don't have any data in them.
>
> So all the index creation/deletion is done between a pair of
> BeginInternalSubTransaction() (BIST()) and
> RollbackAndReleaseCurrentSubTransaction() (RARCST()). This all works well
> when we are EXPLAINing query in special advise mode.
>
> Now, based on this, I wanted to generate advise for every query that
> comes for planning, be it from psql or pl/pgsql or anywhere else. So I made
> a function call to the index_adviser() from within pg_plan_query() after the
> planner() call.
>
> The problem I am facing is that that the queries being planned by
> pl/pgsql, come after a call to SPI_connect() and SPI_Prepare().
> SPI_prepare() switches to _SPI_current->execCxt memory context using
> _SPI_begin_call().
>
> Everything goes well until the index_adviser() calls RARCST() to
> rollback it's SubTransaction. RARCST() ultimately ends up calling
> AtEOSubXact_SPI(), which, just before returning, deletes the execution
> memory context of the surrounding SPI (which happens to be pl/pgsql's SPI).
> This, in effect, frees all the memory allocated by pl/pgsql for the prepare
> stage; and the next time the pl/pgsql's data-structures are referenced, it
> causes a crash. The code in question is:
>
> void
> AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
> {
> bool found = false;
>
> while (_SPI_connected >= 0)
> {
> _SPI_connection *connection = &(_SPI_stack[_SPI_connected]);
>
> if (connection->connectSubid != mySubid)
> break; /* couldn't be any underneath it either */
>
> ......
> ......
> /*
> * If we are aborting a subtransaction and there is an open SPI
> context
> * surrounding the subxact, clean up to prevent memory leakage.
> */
> if (_SPI_current && !isCommit)
> {
> /* free Executor memory the same as _SPI_end_call would do */
> MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
> ^^^^^^^^^^^
> /* throw away any partially created tuple-table */
> SPI_freetuptable(_SPI_current->tuptable);
> _SPI_current->tuptable = NULL;
> }
> }
>
> The code is doing what the comments say, but how I wish it didn't.
> This code assumes that the latest BIST() came from a SPI invoking module,
> which the Index Adviser is not!
>
> I haven't been able to come up with a proof, but I think this
> situation can be simulated/reproduced using some pl/* language that we
> support.
>
> pl/pgsql is safe, since it doesn't allow SAVEPOINT/ROLLBACK TO
> SAVEPOINT inside it's code blocks; but if some pl/xxx allows SAVEPOINTS, I
> think it can be reproduced using something like:
>
> SAVEPOINT xxx ( should call BIST() )
> SAVEPOINT yyy ( should call BIST() )
> Rollback (to yyy) ( should call RARCST() ) will free pl/xxx
> exec context
> Rollback (to xxx) ( should call RARCST() )
>
> Probably just one pair, instead of two nested ones, should reproduce
> it. I tried to reproduce it using pl/pgsql's exception block (there we use
> BIST() and RARCST()); but couldn't succeed.
>
> I hope I have understood the issue correctly, or have I missed
> something? Is there a way to work around this?
>
> Best regards,
>
> --
> gurjeet[(dot)singh](at)EnterpriseDB(dot)com
> singh(dot)gurjeet(at){ gmail | hotmail | yahoo }.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Martijn van Oosterhout 2006-12-03 13:57:08 Re: PostgreSQL win32 fragmentation issue
Previous Message Gurjeet Singh 2006-12-03 11:09:55 Facing a problem with SPI