Re: [GENERAL] Memory Errors...

Started by Tom Laneover 23 years ago17 messages
#1Tom Lane
tgl@sss.pgh.pa.us

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

regards, tom lane

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)

I said:

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.
pltcl_SPI_execp() has a similar problem, and there may be comparable
bugs in other pltcl routines (not to mention other sources of memory
leaks, but I think this is the problem for your example).

I have no time to work on this right now; any volunteers out there?

regards, tom lane

#3Joe Conway
mail@joeconway.com
In reply to: Tom Lane (#1)

Tom Lane wrote:

I said:

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.
pltcl_SPI_execp() has a similar problem, and there may be comparable
bugs in other pltcl routines (not to mention other sources of memory
leaks, but I think this is the problem for your example).

I have no time to work on this right now; any volunteers out there?

I can give it a shot, but probably not until the weekend.

I haven't really followed this thread closely, and don't know tcl very well,
so it would help if someone can send me a minimal tcl function which triggers
the problem.

Thanks,

Joe

#4Nigel J. Andrews
nandrews@investsystems.co.uk
In reply to: Tom Lane (#1)

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Quick, minor point, in the manner of a question:

Why is the pltcl directory called tcl where all the other pls are pl<language>?

That's in src/pl of course. Also in my anoncvs fetch which is a few weeks old
now being from the day before beta freeze.

--
Nigel J. Andrews
Director

---
Logictree Systems Limited
Computer Consultants

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nigel J. Andrews (#4)

Nigel J. Andrews wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Quick, minor point, in the manner of a question:

Why is the pltcl directory called tcl where all the other pls are pl<language>?

I asked the same question a while ago. I asked about changing it but
others didn't want the change. It is hard to rename stuff in CVS and
keep proper history.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nigel J. Andrews (#4)

"Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:

Why is the pltcl directory called tcl where all the other pls are pl<language>?

Consistency? We don't need no steenking consistency!

Personally I'd prefer to remove the pl prefix from the other
subdirectories of src/pl/ ... it seems redundantly wasted excessive
typing ;-) And I'd have preferred to flatten out the src/ subdirectory
of src/pl/[pl]pgsql, which is likewise redundant and inconsistent with
the other PLs.

However, it's fairly painful to make any such change without losing
the CVS version history for the moved files, which is Not a Good Thing.
Or breaking our ability to reconstitute old releases from the CVS tree,
which is Much Worse. So I'm afraid we're stuck with this historical
mischance.

regards, tom lane

#7Nigel J. Andrews
nandrews@investsystems.co.uk
In reply to: Joe Conway (#3)

On Thu, 19 Sep 2002, Joe Conway wrote:

Tom Lane wrote:

I said:

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.
pltcl_SPI_execp() has a similar problem, and there may be comparable
bugs in other pltcl routines (not to mention other sources of memory
leaks, but I think this is the problem for your example).

I have no time to work on this right now; any volunteers out there?

I can give it a shot, but probably not until the weekend.

I haven't really followed this thread closely, and don't know tcl very well,
so it would help if someone can send me a minimal tcl function which triggers
the problem.

I can probably take a look at this tomorrow, already started by looking at the
pltcl_SPI_exec routine. I think a quick glance at ...init_unknown() also shows
a lack of tuptable freeing.

--
Nigel J. Andrews

#8Joe Conway
mail@joeconway.com
In reply to: Nigel J. Andrews (#7)

Nigel J. Andrews wrote:

On Thu, 19 Sep 2002, Joe Conway wrote:

I can give it a shot, but probably not until the weekend.

I haven't really followed this thread closely, and don't know tcl very well,
so it would help if someone can send me a minimal tcl function which triggers
the problem.

I can probably take a look at this tomorrow, already started by looking at the
pltcl_SPI_exec routine. I think a quick glance at ...init_unknown() also shows
a lack of tuptable freeing.

OK -- let me know if you can't find the time and I'll jump back in to it.

Joe

#9Nigel J. Andrews
nandrews@investsystems.co.uk
In reply to: Tom Lane (#1)
1 attachment(s)
Re: [HACKERS] [GENERAL] Memory Errors...

On Thu, 19 Sep 2002, Tom Lane wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

Attached is a patch that frees the SPI_tuptable in all post SPI_exec
non-elog paths in both pltcl_SPI_exec() and pltcl_SPI_execp().

The fault as triggered by the above code has been fixed by this patch but
please read my assumptions below to ensure they are correct.

I have assumed that Tom's comment about this only being required in non-elog
paths is correct, which seems a reasonable assumption to me.

I have also assumed, rather than verified, that freeing the tuptable does
indeed free the tuples as well. Tests with the above function show that the
process does not increase it's memory footprint during it's operation, although
if my assumption here is wrong this could be a feature of selecting
insignificantly sized tuples.

I have not worried about other uses of SPI_exec for selects in pltcl.c on the
basis that those are not under the control of the function writer and the
normal function management will release the storage.

--
Nigel J. Andrews

Attachments:

pltcl.patchtext/plain; charset=US-ASCII; name=pltcl.patchDownload
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/tcl/pltcl.c,v
retrieving revision 1.61
diff -c -r1.61 pltcl.c
*** src/pl/tcl/pltcl.c	2002/09/04 20:31:48	1.61
--- src/pl/tcl/pltcl.c	2002/09/20 11:23:54
***************
*** 1648,1653 ****
--- 1648,1654 ----
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_OK;
  	}
***************
*** 1669,1683 ****
--- 1670,1688 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
  
+ 	SPI_freetuptable(SPI_tuptable);
+ 
  	/************************************************************
  	 * Finally return the number of tuples
  	 ************************************************************/
***************
*** 2208,2213 ****
--- 2213,2219 ----
  	{
  		if (ntuples > 0)
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
***************
*** 2230,2243 ****
--- 2236,2253 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
+ 
+ 	SPI_freetuptable(SPI_tuptable);
  
  	/************************************************************
  	 * Finally return the number of tuples
#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)

Tom Lane writes:

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.

There's a note in the PL/Python documentation that it's leaking memory if
SPI plans are used. Maybe that's related and someone could take a look at
it.

--
Peter Eisentraut peter_e@gmx.net

#11Greg Copeland
greg@CopelandConsulting.Net
In reply to: Peter Eisentraut (#10)

I'll try to have a look-see by the end of the weekend. Any code that
can reproduce it or is it ANY code that uses SPI?

Greg

Show quoted text

On Fri, 2002-09-20 at 11:39, Peter Eisentraut wrote:

Tom Lane writes:

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.

There's a note in the PL/Python documentation that it's leaking memory if
SPI plans are used. Maybe that's related and someone could take a look at
it.

--
Peter Eisentraut peter_e@gmx.net

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

#12Nigel J. Andrews
nandrews@investsystems.co.uk
In reply to: Greg Copeland (#11)

On 20 Sep 2002, Greg Copeland wrote:

I'll try to have a look-see by the end of the weekend. Any code that
can reproduce it or is it ANY code that uses SPI?

Greg

On Fri, 2002-09-20 at 11:39, Peter Eisentraut wrote:

Tom Lane writes:

On looking a little more closely, it's clear that pltcl_SPI_exec()
should be, and is not, calling SPI_freetuptable() once it's done with
the tuple table returned by SPI_exec(). This needs to be done in all
the non-elog code paths after SPI_exec has returned SPI_OK_SELECT.

There's a note in the PL/Python documentation that it's leaking memory if
SPI plans are used. Maybe that's related and someone could take a look at
it.

I've added the call to free the tuptable just as in the pltcl patch I submited
earlier (which I can't remember if I've seen in the list so I may well resend).

However, the comments in the code imply there might be another leak with
prepared plans. I'm looking into that so I won't be sending this patch just
yet.

--
Nigel J. Andrews

#13Nigel J. Andrews
nandrews@investsystems.co.uk
In reply to: Nigel J. Andrews (#9)
3 attachment(s)
Re: Memory Errors...

Ok, below is the original email I sent, which I can not remember seeing come
across the patches list. Please do read the assumptions since they might throw
up problems with what I have done.

I have attached the pltcl patch again, just in case. For the sake of clarity
let's say this patch superscedes the previous one.

I have also attached a patch addressing the similar memory leak problem in
plpython. This includes a slight adjustment of the tests in the source
directory. The patch also includes a cosmetic change to remove a compiler
warning although I think the change makes the code look worse though.

Once again, please read my text below and also take a quick look at the comment
I've added in the plpython patch since it may well show that that
particular change is complete rubbish.

BTW, by my reckoning the memory leak would occur with prepared plans and
without. If that is not the case then I've been barking up the wrong tree.

Of further note, I have not tested for the memory leak in plpython but the
build passes the normal and big checks. However, I have tried testing using the
test.sh script in src/pl/plpython. This seems to be generating errors where
before there were warnings. Can anyone comment on the correctness of this?
Reversing my changes doesn't really help matters so I presume it is something
else that is causing the different behaviour.

--
Nigel J. Andrews

On Fri, 20 Sep 2002, Nigel J. Andrews wrote:

Show quoted text

On Thu, 19 Sep 2002, Tom Lane wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

Attached is a patch that frees the SPI_tuptable in all post SPI_exec
non-elog paths in both pltcl_SPI_exec() and pltcl_SPI_execp().

The fault as triggered by the above code has been fixed by this patch but
please read my assumptions below to ensure they are correct.

I have assumed that Tom's comment about this only being required in non-elog
paths is correct, which seems a reasonable assumption to me.

I have also assumed, rather than verified, that freeing the tuptable does
indeed free the tuples as well. Tests with the above function show that the
process does not increase it's memory footprint during it's operation, although
if my assumption here is wrong this could be a feature of selecting
insignificantly sized tuples.

I have not worried about other uses of SPI_exec for selects in pltcl.c on the
basis that those are not under the control of the function writer and the
normal function management will release the storage.

Attachments:

plpython.patchtext/plain; charset=US-ASCII; name=plpython.patchDownload
Index: src/pl/plpython/feature.expected
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/plpython/feature.expected,v
retrieving revision 1.4
diff -c -r1.4 feature.expected
*** src/pl/plpython/feature.expected	2002/03/06 18:50:31	1.4
--- src/pl/plpython/feature.expected	2002/09/20 22:12:36
***************
*** 29,35 ****
  (1 row)
  
  SELECT import_fail();
! WARNING:  ('import socket failed -- untrusted dynamic module: _socket',)
      import_fail     
  --------------------
   failed as expected
--- 29,35 ----
  (1 row)
  
  SELECT import_fail();
! NOTICE:  ('import socket failed -- untrusted dynamic module: _socket',)
      import_fail     
  --------------------
   failed as expected
Index: src/pl/plpython/plpython.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/plpython/plpython.c,v
retrieving revision 1.22
diff -c -r1.22 plpython.c
*** src/pl/plpython/plpython.c	2002/09/04 22:51:23	1.22
--- src/pl/plpython/plpython.c	2002/09/20 22:12:40
***************
*** 408,414 ****
--- 408,416 ----
  		else
  			PLy_restart_in_progress += 1;
  		if (proc)
+ 		{
  			Py_DECREF(proc->me);
+ 		}
  		RERAISE_EXC();
  	}
  
***************
*** 1841,1847 ****
--- 1843,1856 ----
  		 *
  		 * FIXME -- leaks saved plan on object destruction.  can this be
  		 * avoided?
+ 		 * I think so. A function prepares and then execp's a statement.
+ 		 * When we come to deallocate the 'statement' object we obviously
+ 		 * no long need the plan. Even if we did, without the object
+ 		 * we're never going to be able to use it again.
+ 		 * In the against arguments: SPI_saveplan has stuck this under
+ 		 * the top context so there must be a reason for doing that.
  		 */
+ 		pfree(ob->plan);
  	}
  	if (ob->types)
  		PLy_free(ob->types);
***************
*** 2374,2379 ****
--- 2383,2390 ----
  				PyList_SetItem(result->rows, i, row);
  			}
  			PLy_typeinfo_dealloc(&args);
+ 
+ 			SPI_freetuptable(tuptable);
  		}
  		RESTORE_EXC();
  	}
Index: src/pl/plpython/plpython_schema.sql
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/plpython/plpython_schema.sql,v
retrieving revision 1.1
diff -c -r1.1 plpython_schema.sql
*** src/pl/plpython/plpython_schema.sql	2001/05/09 19:54:38	1.1
--- src/pl/plpython/plpython_schema.sql	2002/09/20 22:12:40
***************
*** 20,26 ****
  
  CREATE TABLE entry (
  	accession text not null primary key,
! 	eid serial,
  	txid int2 not null references taxonomy(id)
  	) ;
  
--- 20,26 ----
  
  CREATE TABLE entry (
  	accession text not null primary key,
! 	eid serial unique,
  	txid int2 not null references taxonomy(id)
  	) ;
  
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/tcl/pltcl.c,v
retrieving revision 1.61
diff -c -r1.61 pltcl.c
*** src/pl/tcl/pltcl.c	2002/09/04 20:31:48	1.61
--- src/pl/tcl/pltcl.c	2002/09/20 22:12:42
***************
*** 1648,1653 ****
--- 1648,1654 ----
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_OK;
  	}
***************
*** 1669,1683 ****
--- 1670,1688 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
  
+ 	SPI_freetuptable(SPI_tuptable);
+ 
  	/************************************************************
  	 * Finally return the number of tuples
  	 ************************************************************/
***************
*** 2208,2213 ****
--- 2213,2219 ----
  	{
  		if (ntuples > 0)
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
***************
*** 2230,2243 ****
--- 2236,2253 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
+ 
+ 	SPI_freetuptable(SPI_tuptable);
  
  	/************************************************************
  	 * Finally return the number of tuples
pltcl.patchtext/plain; charset=US-ASCII; name=pltcl.patchDownload
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/pl/tcl/pltcl.c,v
retrieving revision 1.61
diff -c -r1.61 pltcl.c
*** src/pl/tcl/pltcl.c	2002/09/04 20:31:48	1.61
--- src/pl/tcl/pltcl.c	2002/09/20 22:12:42
***************
*** 1648,1653 ****
--- 1648,1654 ----
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_OK;
  	}
***************
*** 1669,1683 ****
--- 1670,1688 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
  
+ 	SPI_freetuptable(SPI_tuptable);
+ 
  	/************************************************************
  	 * Finally return the number of tuples
  	 ************************************************************/
***************
*** 2208,2213 ****
--- 2213,2219 ----
  	{
  		if (ntuples > 0)
  			pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		sprintf(buf, "%d", ntuples);
  		Tcl_SetResult(interp, buf, TCL_VOLATILE);
***************
*** 2230,2243 ****
--- 2236,2253 ----
  			continue;
  		if (loop_rc == TCL_RETURN)
  		{
+ 			SPI_freetuptable(SPI_tuptable);
  			memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  			return TCL_RETURN;
  		}
  		if (loop_rc == TCL_BREAK)
  			break;
+ 		SPI_freetuptable(SPI_tuptable);
  		memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
  		return TCL_ERROR;
  	}
+ 
+ 	SPI_freetuptable(SPI_tuptable);
  
  	/************************************************************
  	 * Finally return the number of tuples
error.difftext/plain; charset=US-ASCII; name=error.diffDownload
--- error.expected	Wed Mar  6 18:50:31 2002
+++ error.output	Fri Sep 20 23:08:08 2002
@@ -1,12 +1,15 @@
 SELECT invalid_type_uncaught('rick');
-WARNING:  plpython: in function __plpython_procedure_invalid_type_uncaught_49801:
-plpy.SPIError: Cache lookup for type `test' failed.
+WARNING:  plpython: in function __plpython_procedure_invalid_type_uncaught_17083:
+plpy.SPIError: Unknown error in PLy_spi_prepare.
+ERROR:  Type "test" does not exist
 SELECT invalid_type_caught('rick');
-WARNING:  plpython: in function __plpython_procedure_invalid_type_caught_49802:
-plpy.SPIError: Cache lookup for type `test' failed.
+WARNING:  plpython: in function __plpython_procedure_invalid_type_caught_17084:
+plpy.SPIError: Unknown error in PLy_spi_prepare.
+ERROR:  Type "test" does not exist
 SELECT invalid_type_reraised('rick');
-WARNING:  plpython: in function __plpython_procedure_invalid_type_reraised_49803:
-plpy.SPIError: Cache lookup for type `test' failed.
+WARNING:  plpython: in function __plpython_procedure_invalid_type_reraised_17085:
+plpy.SPIError: Unknown error in PLy_spi_prepare.
+ERROR:  Type "test" does not exist
 SELECT valid_type('rick');
  valid_type 
 ------------
@@ -14,19 +17,19 @@
 (1 row)
 
 SELECT read_file('/etc/passwd');
-ERROR:  plpython: Call of function `__plpython_procedure_read_file_49809' failed.
+ERROR:  plpython: Call of function `__plpython_procedure_read_file_17091' failed.
 exceptions.IOError: can't open files in restricted mode
 SELECT write_file('/tmp/plpython','This is very bad');
-ERROR:  plpython: Call of function `__plpython_procedure_write_file_49810' failed.
+ERROR:  plpython: Call of function `__plpython_procedure_write_file_17092' failed.
 exceptions.IOError: can't open files in restricted mode
 SELECT getpid();
-ERROR:  plpython: Call of function `__plpython_procedure_getpid_49811' failed.
+ERROR:  plpython: Call of function `__plpython_procedure_getpid_17093' failed.
 exceptions.AttributeError: getpid
 SELECT uname();
-ERROR:  plpython: Call of function `__plpython_procedure_uname_49812' failed.
+ERROR:  plpython: Call of function `__plpython_procedure_uname_17094' failed.
 exceptions.AttributeError: uname
 SELECT sys_exit();
-ERROR:  plpython: Call of function `__plpython_procedure_sys_exit_49813' failed.
+ERROR:  plpython: Call of function `__plpython_procedure_sys_exit_17095' failed.
 exceptions.AttributeError: exit
 SELECT sys_argv();
     sys_argv    
#14Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nigel J. Andrews (#9)
Re: [HACKERS] [GENERAL] Memory Errors...

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Nigel J. Andrews wrote:

On Thu, 19 Sep 2002, Tom Lane wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

Attached is a patch that frees the SPI_tuptable in all post SPI_exec
non-elog paths in both pltcl_SPI_exec() and pltcl_SPI_execp().

The fault as triggered by the above code has been fixed by this patch but
please read my assumptions below to ensure they are correct.

I have assumed that Tom's comment about this only being required in non-elog
paths is correct, which seems a reasonable assumption to me.

I have also assumed, rather than verified, that freeing the tuptable does
indeed free the tuples as well. Tests with the above function show that the
process does not increase it's memory footprint during it's operation, although
if my assumption here is wrong this could be a feature of selecting
insignificantly sized tuples.

I have not worried about other uses of SPI_exec for selects in pltcl.c on the
basis that those are not under the control of the function writer and the
normal function management will release the storage.

--
Nigel J. Andrews

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#15Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nigel J. Andrews (#13)
Re: Memory Errors...

[ Previous version removed from patches queue..]

Thanks for doing both interfaces.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Nigel J. Andrews wrote:

Ok, below is the original email I sent, which I can not remember seeing come
across the patches list. Please do read the assumptions since they might throw
up problems with what I have done.

I have attached the pltcl patch again, just in case. For the sake of clarity
let's say this patch superscedes the previous one.

I have also attached a patch addressing the similar memory leak problem in
plpython. This includes a slight adjustment of the tests in the source
directory. The patch also includes a cosmetic change to remove a compiler
warning although I think the change makes the code look worse though.

Once again, please read my text below and also take a quick look at the comment
I've added in the plpython patch since it may well show that that
particular change is complete rubbish.

BTW, by my reckoning the memory leak would occur with prepared plans and
without. If that is not the case then I've been barking up the wrong tree.

Of further note, I have not tested for the memory leak in plpython but the
build passes the normal and big checks. However, I have tried testing using the
test.sh script in src/pl/plpython. This seems to be generating errors where
before there were warnings. Can anyone comment on the correctness of this?
Reversing my changes doesn't really help matters so I presume it is something
else that is causing the different behaviour.

--
Nigel J. Andrews

On Fri, 20 Sep 2002, Nigel J. Andrews wrote:

On Thu, 19 Sep 2002, Tom Lane wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

Attached is a patch that frees the SPI_tuptable in all post SPI_exec
non-elog paths in both pltcl_SPI_exec() and pltcl_SPI_execp().

The fault as triggered by the above code has been fixed by this patch but
please read my assumptions below to ensure they are correct.

I have assumed that Tom's comment about this only being required in non-elog
paths is correct, which seems a reasonable assumption to me.

I have also assumed, rather than verified, that freeing the tuptable does
indeed free the tuples as well. Tests with the above function show that the
process does not increase it's memory footprint during it's operation, although
if my assumption here is wrong this could be a feature of selecting
insignificantly sized tuples.

I have not worried about other uses of SPI_exec for selects in pltcl.c on the
basis that those are not under the control of the function writer and the
normal function management will release the storage.

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#16Greg Copeland
greg@CopelandConsulting.Net
In reply to: Joe Conway (#8)

Well, it looks like it was already taken to the mat.

;)

Greg

Show quoted text

On Thu, 2002-09-19 at 16:58, Joe Conway wrote:

Nigel J. Andrews wrote:

On Thu, 19 Sep 2002, Joe Conway wrote:

I can give it a shot, but probably not until the weekend.

I haven't really followed this thread closely, and don't know tcl very well,
so it would help if someone can send me a minimal tcl function which triggers
the problem.

I can probably take a look at this tomorrow, already started by looking at the
pltcl_SPI_exec routine. I think a quick glance at ...init_unknown() also shows
a lack of tuptable freeing.

OK -- let me know if you can't find the time and I'll jump back in to it.

Joe

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/users-lounge/docs/faq.html

#17Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Nigel J. Andrews (#13)
Re: Memory Errors...

Patch applied. Thanks.

---------------------------------------------------------------------------

Nigel J. Andrews wrote:

Ok, below is the original email I sent, which I can not remember seeing come
across the patches list. Please do read the assumptions since they might throw
up problems with what I have done.

I have attached the pltcl patch again, just in case. For the sake of clarity
let's say this patch superscedes the previous one.

I have also attached a patch addressing the similar memory leak problem in
plpython. This includes a slight adjustment of the tests in the source
directory. The patch also includes a cosmetic change to remove a compiler
warning although I think the change makes the code look worse though.

Once again, please read my text below and also take a quick look at the comment
I've added in the plpython patch since it may well show that that
particular change is complete rubbish.

BTW, by my reckoning the memory leak would occur with prepared plans and
without. If that is not the case then I've been barking up the wrong tree.

Of further note, I have not tested for the memory leak in plpython but the
build passes the normal and big checks. However, I have tried testing using the
test.sh script in src/pl/plpython. This seems to be generating errors where
before there were warnings. Can anyone comment on the correctness of this?
Reversing my changes doesn't really help matters so I presume it is something
else that is causing the different behaviour.

--
Nigel J. Andrews

On Fri, 20 Sep 2002, Nigel J. Andrews wrote:

On Thu, 19 Sep 2002, Tom Lane wrote:

"Ian Harding" <ianh@tpchd.org> writes:

It is pltcl [not plpgsql]

Ah. I don't think we've done much of any work on plugging leaks in
pltcl :-(.

It hurts when I do this:

drop function memleak();
create function memleak() returns int as '
for {set counter 1} {$counter < 100000} {incr counter} {
set sql "select ''foo''"
spi_exec "$sql"
}
' language 'pltcl';
select memleak();

Yeah, I see very quick memory exhaustion also :-(. Looks like the
spi_exec call is the culprit, but I'm not sure exactly why ...
anyone have time to look at this?

Attached is a patch that frees the SPI_tuptable in all post SPI_exec
non-elog paths in both pltcl_SPI_exec() and pltcl_SPI_execp().

The fault as triggered by the above code has been fixed by this patch but
please read my assumptions below to ensure they are correct.

I have assumed that Tom's comment about this only being required in non-elog
paths is correct, which seems a reasonable assumption to me.

I have also assumed, rather than verified, that freeing the tuptable does
indeed free the tuples as well. Tests with the above function show that the
process does not increase it's memory footprint during it's operation, although
if my assumption here is wrong this could be a feature of selecting
insignificantly sized tuples.

I have not worried about other uses of SPI_exec for selects in pltcl.c on the
basis that those are not under the control of the function writer and the
normal function management will release the storage.

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073