what about just free _SPI_stack in AtEOXact_SPI? if the transaction end was initiated by SPI , AtEOXact_SPI will do nothing. for example: @@ -283,6 +295,8 @@ AtEOXact_SPI(bool isCommit) errmsg("transaction left non-empty SPI stack"), errhint("Check for missing \"SPI_finish\" calls.")));
+ if (_SPI_stack) + pfree(_SPI_stack); From: Michael Paquier Date: 2018-04-20 08:49 To: Postgres hackers CC: Peter Eisentraut; Andrew Dunstan; jian.l...@i-soft.com.cn Subject: Re: Is there a memory leak in commit 8561e48? On Thu, Apr 19, 2018 at 03:10:42PM +0900, Michael Paquier wrote: > You are right. I can easily see the leak if I use for example a > background worker which connects to a database, and launches many > transactions in a row. The laziest reproducer I have is to patch one of > my bgworkers to launch millions of transactions in a tight loop and the > leak is plain (this counts relations automatically, does not matter): > https://github.com/michaelpq/pg_plugins/tree/master/count_relations > > TopMemoryContext is associated to a session, so the comment in > AtEOXact_SPI() is wrong. I have been looking at this one this morning, and I can see at least two problems: 1) When SPI_connect_ext is used in an atomic context, then the allocation of _SPI_stack should happen in TopTransactionContext instead of TopMemoryContext. This way, any callers of SPI_connect would not be impacted by any memory leaks. 2) Error stacks with non-atomic calls leak memorya anyway. It is easy to find leaks of _SPI_stack in the regression tests when an ERROR happens in a PL call which lead to AtEOXact_SPI being called in AbortTransaction. See that as an example: @@ -283,6 +285,12 @@ AtEOXact_SPI(bool isCommit) errmsg("transaction left non-empty SPI stack"), errhint("Check for missing \"SPI_finish\" calls."))); + if (_SPI_current != NULL && !_SPI_current->atomic && _SPI_stack != NULL) + ereport(WARNING, + (errcode(ERRCODE_WARNING), + errmsg("non-atomic transaction left non-empty SPI stack"), + errhint("Check after non-atomic \"SPI_connect_ext\" calls."))); The cleanest approach I can think about is to have SPI use its own memory context which gets cleaned up in AtEOXact_SPI, but I would like to hear more from Peter Eisentraut and Andrew Dunstand first as author/committer and reviewer as it is their stuff. I am attaching a preliminary patch, which fixes partially the leak, but that does not take care of the leaks caused by the error stacks. -- Michael