"Nigel J. Andrews" <[EMAIL PROTECTED]> writes:
> Yes, I do get the similar results.
>
> A quick investigation shows that the SPI_freetuptable at the end of
> pltcl_SPI_exec is trying to free a tuptable of value 0x82ebe64
> (which looks sensible to me) but which has a memory context of
> 0x7f7f7f7f (the unallocated marker).
Attached is a patch against CVS HEAD which fixes this, I believe. The
problem appears to be the newly added free of the tuptable at the end
of pltcl_SPI_exec(). I've added a comment to that effect:
/*
* Do *NOT* free the tuptable here. That's because if the loop
* body executed any SQL statements, it will have already free'd
* the tuptable itself, so freeing it twice is not wise. We could
* get around this by making a copy of SPI_tuptable->vals and
* feeding that to pltcl_set_tuple_values above, but that would
* still leak memory (the palloc'ed copy would only be free'd on
* context reset).
*/
At least, I *think* that's the problem -- I've only been looking at
the code for about 20 minutes, so I may be wrong. In any case, this
makes both memleak() and memleak(1) work on my machine. Let me know if
it works for you, and/or if someone knows of a better solution.
I also added some SPI_freetuptable() calls in some places where Nigel
didn't, and added some paranoia when dealing with statically sized
buffers (snprintf() rather than sprintf(), and so on). I also didn't
include Nigel's changes to some apparently unrelated PL/Python stuff
-- this patch includes only the PL/Tcl changes.
Cheers,
Neil
--
Neil Conway <[EMAIL PROTECTED]> || PGP Key ID: DB3C29FC
Index: src/pl/tcl/pltcl.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/pl/tcl/pltcl.c,v
retrieving revision 1.62
diff -c -r1.62 pltcl.c
*** src/pl/tcl/pltcl.c 21 Sep 2002 18:39:26 -0000 1.62
--- src/pl/tcl/pltcl.c 25 Sep 2002 04:54:57 -0000
***************
*** 39,50 ****
#include <tcl.h>
- #include <stdio.h>
- #include <stdlib.h>
- #include <stdarg.h>
#include <unistd.h>
#include <fcntl.h>
- #include <string.h>
#include <setjmp.h>
#include "access/heapam.h"
--- 39,46 ----
***************
*** 308,313 ****
--- 304,310 ----
************************************************************/
spi_rc = SPI_exec("select 1 from pg_class "
"where relname = 'pltcl_modules'", 1);
+ SPI_freetuptable(SPI_tuptable);
if (spi_rc != SPI_OK_SELECT)
elog(ERROR, "pltcl_init_load_unknown(): select from pg_class failed");
if (SPI_processed == 0)
***************
*** 334,339 ****
--- 331,337 ----
if (SPI_processed == 0)
{
Tcl_DStringFree(&unknown_src);
+ SPI_freetuptable(SPI_tuptable);
elog(WARNING, "pltcl: Module unknown not found in pltcl_modules");
return;
}
***************
*** 359,364 ****
--- 357,363 ----
}
tcl_rc = Tcl_GlobalEval(interp, Tcl_DStringValue(&unknown_src));
Tcl_DStringFree(&unknown_src);
+ SPI_freetuptable(SPI_tuptable);
}
***************
*** 955,963 ****
* Build our internal proc name from the functions Oid
************************************************************/
if (!is_trigger)
! sprintf(internal_proname, "__PLTcl_proc_%u", fn_oid);
else
! sprintf(internal_proname, "__PLTcl_proc_%u_trigger", fn_oid);
/************************************************************
* Lookup the internal proc name in the hashtable
--- 954,964 ----
* Build our internal proc name from the functions Oid
************************************************************/
if (!is_trigger)
! snprintf(internal_proname, sizeof(internal_proname),
! "__PLTcl_proc_%u", fn_oid);
else
! snprintf(internal_proname, sizeof(internal_proname),
! "__PLTcl_proc_%u_trigger", fn_oid);
/************************************************************
* Lookup the internal proc name in the hashtable
***************
*** 1127,1133 ****
prodesc->arg_is_rel[i] = 1;
if (i > 0)
strcat(proc_internal_args, " ");
! sprintf(buf, "__PLTcl_Tup_%d", i + 1);
strcat(proc_internal_args, buf);
ReleaseSysCache(typeTup);
continue;
--- 1128,1134 ----
prodesc->arg_is_rel[i] = 1;
if (i > 0)
strcat(proc_internal_args, " ");
! snprintf(buf, sizeof(buf), "__PLTcl_Tup_%d", i + 1);
strcat(proc_internal_args, buf);
ReleaseSysCache(typeTup);
continue;
***************
*** 1140,1146 ****
if (i > 0)
strcat(proc_internal_args, " ");
! sprintf(buf, "%d", i + 1);
strcat(proc_internal_args, buf);
ReleaseSysCache(typeTup);
--- 1141,1147 ----
if (i > 0)
strcat(proc_internal_args, " ");
! snprintf(buf, sizeof(buf), "%d", i + 1);
strcat(proc_internal_args, buf);
ReleaseSysCache(typeTup);
***************
*** 1177,1183 ****
{
if (!prodesc->arg_is_rel[i])
continue;
! sprintf(buf, "array set %d $__PLTcl_Tup_%d\n", i + 1, i + 1);
Tcl_DStringAppend(&proc_internal_body, buf, -1);
}
}
--- 1178,1185 ----
{
if (!prodesc->arg_is_rel[i])
continue;
! snprintf(buf, sizeof(buf), "array set %d $__PLTcl_Tup_%d\n",
! i + 1, i + 1);
Tcl_DStringAppend(&proc_internal_body, buf, -1);
}
}
***************
*** 1557,1570 ****
{
case SPI_OK_UTILITY:
Tcl_SetResult(interp, "0", TCL_VOLATILE);
return TCL_OK;
case SPI_OK_SELINTO:
case SPI_OK_INSERT:
case SPI_OK_DELETE:
case SPI_OK_UPDATE:
! sprintf(buf, "%d", SPI_processed);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
case SPI_OK_SELECT:
--- 1559,1574 ----
{
case SPI_OK_UTILITY:
Tcl_SetResult(interp, "0", TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
return TCL_OK;
case SPI_OK_SELINTO:
case SPI_OK_INSERT:
case SPI_OK_DELETE:
case SPI_OK_UPDATE:
! snprintf(buf, sizeof(buf), "%d", SPI_processed);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
return TCL_OK;
case SPI_OK_SELECT:
***************
*** 1607,1613 ****
return TCL_ERROR;
default:
! sprintf(buf, "%d", spi_rc);
Tcl_AppendResult(interp, "pltcl: SPI_exec() failed - ",
"unknown RC ", buf, NULL);
return TCL_ERROR;
--- 1611,1617 ----
return TCL_ERROR;
default:
! snprintf(buf, sizeof(buf), "%d", spi_rc);
Tcl_AppendResult(interp, "pltcl: SPI_exec() failed - ",
"unknown RC ", buf, NULL);
return TCL_ERROR;
***************
*** 1645,1652 ****
{
if (ntuples > 0)
pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
! sprintf(buf, "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
return TCL_OK;
}
--- 1649,1657 ----
{
if (ntuples > 0)
pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
! snprintf(buf, sizeof(buf), "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
return TCL_OK;
}
***************
*** 1668,1678 ****
--- 1673,1685 ----
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;
}
***************
*** 1681,1688 ****
* Finally return the number of tuples
************************************************************/
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! sprintf(buf, "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
}
--- 1688,1705 ----
* Finally return the number of tuples
************************************************************/
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! snprintf(buf, sizeof(buf), "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+
+ /*
+ * Do *NOT* free the tuptable here. That's because if the loop body
+ * executed any SQL statements, it will have already free'd the
+ * tuptable itself, so freeing it twice is not wise. We could get
+ * around this by making a copy of SPI_tuptable->vals and feeding
+ * that to pltcl_set_tuple_values above, but that would still leak
+ * memory (the palloc'ed copy would only be free'd on context
+ * reset).
+ */
return TCL_OK;
}
***************
*** 1690,1696 ****
/**********************************************************************
* pltcl_SPI_prepare() - Builtin support for prepared plans
* The Tcl command SPI_prepare
! * allways saves the plan using
* SPI_saveplan and returns a key for
* access. There is no chance to prepare
* and not save the plan currently.
--- 1707,1713 ----
/**********************************************************************
* pltcl_SPI_prepare() - Builtin support for prepared plans
* The Tcl command SPI_prepare
! * always saves the plan using
* SPI_saveplan and returns a key for
* access. There is no chance to prepare
* and not save the plan currently.
***************
*** 1736,1742 ****
* Allocate the new querydesc structure
************************************************************/
qdesc = (pltcl_query_desc *) malloc(sizeof(pltcl_query_desc));
! sprintf(qdesc->qname, "%lx", (long) qdesc);
qdesc->nargs = nargs;
qdesc->argtypes = (Oid *) malloc(nargs * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) malloc(nargs * sizeof(FmgrInfo));
--- 1753,1759 ----
* Allocate the new querydesc structure
************************************************************/
qdesc = (pltcl_query_desc *) malloc(sizeof(pltcl_query_desc));
! snprintf(qdesc->qname, sizeof(qdesc->qname), "%lx", (long) qdesc);
qdesc->nargs = nargs;
qdesc->argtypes = (Oid *) malloc(nargs * sizeof(Oid));
qdesc->arginfuncs = (FmgrInfo *) malloc(nargs * sizeof(FmgrInfo));
***************
*** 1815,1821 ****
break;
default:
! sprintf(buf, "unknown RC %d", SPI_result);
reason = buf;
break;
--- 1832,1838 ----
break;
default:
! snprintf(buf, sizeof(buf), "unknown RC %d", SPI_result);
reason = buf;
break;
***************
*** 2116,2129 ****
{
case SPI_OK_UTILITY:
Tcl_SetResult(interp, "0", TCL_VOLATILE);
return TCL_OK;
case SPI_OK_SELINTO:
case SPI_OK_INSERT:
case SPI_OK_DELETE:
case SPI_OK_UPDATE:
! sprintf(buf, "%d", SPI_processed);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
case SPI_OK_SELECT:
--- 2133,2148 ----
{
case SPI_OK_UTILITY:
Tcl_SetResult(interp, "0", TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
return TCL_OK;
case SPI_OK_SELINTO:
case SPI_OK_INSERT:
case SPI_OK_DELETE:
case SPI_OK_UPDATE:
! snprintf(buf, sizeof(buf), "%d", SPI_processed);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
return TCL_OK;
case SPI_OK_SELECT:
***************
*** 2166,2172 ****
return TCL_ERROR;
default:
! sprintf(buf, "%d", spi_rc);
Tcl_AppendResult(interp, "pltcl: SPI_exec() failed - ",
"unknown RC ", buf, NULL);
return TCL_ERROR;
--- 2185,2191 ----
return TCL_ERROR;
default:
! snprintf(buf, sizeof(buf), "%d", spi_rc);
Tcl_AppendResult(interp, "pltcl: SPI_exec() failed - ",
"unknown RC ", buf, NULL);
return TCL_ERROR;
***************
*** 2208,2215 ****
if (ntuples > 0)
pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! sprintf(buf, "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
}
--- 2227,2235 ----
if (ntuples > 0)
pltcl_set_tuple_values(interp, arrayname, 0, tuples[0], tupdesc);
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! snprintf(buf, sizeof(buf), "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+ SPI_freetuptable(SPI_tuptable);
return TCL_OK;
}
***************
*** 2229,2239 ****
--- 2249,2261 ----
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;
}
***************
*** 2242,2249 ****
* Finally return the number of tuples
************************************************************/
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! sprintf(buf, "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
}
--- 2264,2281 ----
* Finally return the number of tuples
************************************************************/
memcpy(&Warn_restart, &save_restart, sizeof(Warn_restart));
! snprintf(buf, sizeof(buf), "%d", ntuples);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
+
+ /*
+ * Do *NOT* free the tuptable here. That's because if the loop body
+ * executed any SQL statements, it will have already free'd the
+ * tuptable itself, so freeing it twice is not wise. We could get
+ * around this by making a copy of SPI_tuptable->vals and feeding
+ * that to pltcl_set_tuple_values above, but that would still leak
+ * memory (the palloc'ed copy would only be free'd on context
+ * reset).
+ */
return TCL_OK;
}
***************
*** 2258,2264 ****
{
char buf[64];
! sprintf(buf, "%u", SPI_lastoid);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
}
--- 2290,2296 ----
{
char buf[64];
! snprintf(buf, sizeof(buf), "%u", SPI_lastoid);
Tcl_SetResult(interp, buf, TCL_VOLATILE);
return TCL_OK;
}
***************
*** 2300,2306 ****
{
arrptr = &arrayname;
nameptr = &attname;
! sprintf(buf, "%d", tupno);
Tcl_SetVar2(interp, arrayname, ".tupno", buf, 0);
}
--- 2332,2338 ----
{
arrptr = &arrayname;
nameptr = &attname;
! snprintf(buf, sizeof(buf), "%d", tupno);
Tcl_SetVar2(interp, arrayname, ".tupno", buf, 0);
}
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
