Commit 8c75ad43 authored by Tom Lane's avatar Tom Lane

Fix memory leaks in PL/Python.

Previously, plpython was in the habit of allocating a lot of stuff in
TopMemoryContext, and it was very slipshod about making sure that stuff
got cleaned up; in particular, use of TopMemoryContext as fn_mcxt for
function calls represents an unfixable leak, since we generally don't
know what the called function might have allocated in fn_mcxt.  This
results in session-lifespan leakage in certain usage scenarios, as for
example in a case reported by Ed Behn back in July.

To fix, get rid of all the retail allocations in TopMemoryContext.
All long-lived allocations are now made in sub-contexts that are
associated with specific objects (either pl/python procedures, or
Python-visible objects such as cursors and plans).  We can clean these
up when the associated object is deleted.

I went so far as to get rid of PLy_malloc completely.  There were a
couple of places where it could still have been used safely, but on
the whole it was just an invitation to bad coding.

Haribabu Kommi, based on a draft patch by Heikki Linnakangas;
some further work by me
parent 64b2e7ad
......@@ -8,6 +8,7 @@
#include "access/xact.h"
#include "mb/pg_wchar.h"
#include "utils/memutils.h"
#include "plpython.h"
......@@ -111,7 +112,12 @@ PLy_cursor_query(const char *query)
return NULL;
cursor->portalname = NULL;
cursor->closed = false;
PLy_typeinfo_init(&cursor->result);
cursor->mcxt = AllocSetContextCreate(TopMemoryContext,
"PL/Python cursor context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
PLy_typeinfo_init(&cursor->result, cursor->mcxt);
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
......@@ -139,7 +145,7 @@ PLy_cursor_query(const char *query)
elog(ERROR, "SPI_cursor_open() failed: %s",
SPI_result_code_string(SPI_result));
cursor->portalname = PLy_strdup(portal->name);
cursor->portalname = MemoryContextStrdup(cursor->mcxt, portal->name);
PLy_spi_subtransaction_commit(oldcontext, oldowner);
}
......@@ -200,7 +206,12 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
return NULL;
cursor->portalname = NULL;
cursor->closed = false;
PLy_typeinfo_init(&cursor->result);
cursor->mcxt = AllocSetContextCreate(TopMemoryContext,
"PL/Python cursor context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
PLy_typeinfo_init(&cursor->result, cursor->mcxt);
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
......@@ -261,7 +272,7 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
elog(ERROR, "SPI_cursor_open() failed: %s",
SPI_result_code_string(SPI_result));
cursor->portalname = PLy_strdup(portal->name);
cursor->portalname = MemoryContextStrdup(cursor->mcxt, portal->name);
PLy_spi_subtransaction_commit(oldcontext, oldowner);
}
......@@ -315,12 +326,13 @@ PLy_cursor_dealloc(PyObject *arg)
if (PortalIsValid(portal))
SPI_cursor_close(portal);
cursor->closed = true;
}
if (cursor->mcxt)
{
MemoryContextDelete(cursor->mcxt);
cursor->mcxt = NULL;
}
PLy_free(cursor->portalname);
cursor->portalname = NULL;
PLy_typeinfo_dealloc(&cursor->result);
arg->ob_type->tp_free(arg);
}
......
......@@ -14,6 +14,7 @@ typedef struct PLyCursorObject
char *portalname;
PLyTypeInfo result;
bool closed;
MemoryContext mcxt;
} PLyCursorObject;
extern void PLy_cursor_init_type(void);
......
......@@ -852,6 +852,6 @@ PLy_abort_open_subtransactions(int save_subxact_level)
MemoryContextSwitchTo(subtransactiondata->oldcontext);
CurrentResourceOwner = subtransactiondata->oldowner;
PLy_free(subtransactiondata);
pfree(subtransactiondata);
}
}
......@@ -277,7 +277,12 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
flinfo.fn_mcxt = CurrentMemoryContext;
MemSet(&proc, 0, sizeof(PLyProcedure));
proc.pyname = PLy_strdup("__plpython_inline_block");
proc.mcxt = AllocSetContextCreate(TopMemoryContext,
"__plpython_inline_block",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
proc.pyname = MemoryContextStrdup(proc.mcxt, "__plpython_inline_block");
proc.langid = codeblock->langOid;
proc.result.out.d.typoid = VOIDOID;
......@@ -364,17 +369,32 @@ PLy_current_execution_context(void)
return PLy_execution_contexts;
}
static PLyExecutionContext *
PLy_push_execution_context(void)
MemoryContext
PLy_get_scratch_context(PLyExecutionContext *context)
{
PLyExecutionContext *context = PLy_malloc(sizeof(PLyExecutionContext));
context->curr_proc = NULL;
context->scratch_ctx = AllocSetContextCreate(TopTransactionContext,
/*
* A scratch context might never be needed in a given plpython procedure,
* so allocate it on first request.
*/
if (context->scratch_ctx == NULL)
context->scratch_ctx =
AllocSetContextCreate(TopTransactionContext,
"PL/Python scratch context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
return context->scratch_ctx;
}
static PLyExecutionContext *
PLy_push_execution_context(void)
{
PLyExecutionContext *context;
context = (PLyExecutionContext *)
MemoryContextAlloc(TopTransactionContext, sizeof(PLyExecutionContext));
context->curr_proc = NULL;
context->scratch_ctx = NULL;
context->next = PLy_execution_contexts;
PLy_execution_contexts = context;
return context;
......@@ -390,6 +410,7 @@ PLy_pop_execution_context(void)
PLy_execution_contexts = context->next;
if (context->scratch_ctx)
MemoryContextDelete(context->scratch_ctx);
PLy_free(context);
pfree(context);
}
......@@ -25,4 +25,7 @@ typedef struct PLyExecutionContext
/* Get the current execution context */
extern PLyExecutionContext *PLy_current_execution_context(void);
/* Get the scratch memory context for specified execution context */
extern MemoryContext PLy_get_scratch_context(PLyExecutionContext *context);
#endif /* PLPY_MAIN_H */
......@@ -11,6 +11,7 @@
#include "plpy_planobject.h"
#include "plpy_elog.h"
#include "utils/memutils.h"
static void PLy_plan_dealloc(PyObject *arg);
......@@ -80,6 +81,7 @@ PLy_plan_new(void)
ob->types = NULL;
ob->values = NULL;
ob->args = NULL;
ob->mcxt = NULL;
return (PyObject *) ob;
}
......@@ -96,20 +98,15 @@ PLy_plan_dealloc(PyObject *arg)
PLyPlanObject *ob = (PLyPlanObject *) arg;
if (ob->plan)
{
SPI_freeplan(ob->plan);
if (ob->types)
PLy_free(ob->types);
if (ob->values)
PLy_free(ob->values);
if (ob->args)
ob->plan = NULL;
}
if (ob->mcxt)
{
int i;
for (i = 0; i < ob->nargs; i++)
PLy_typeinfo_dealloc(&ob->args[i]);
PLy_free(ob->args);
MemoryContextDelete(ob->mcxt);
ob->mcxt = NULL;
}
arg->ob_type->tp_free(arg);
}
......
......@@ -17,6 +17,7 @@ typedef struct PLyPlanObject
Oid *types;
Datum *values;
PLyTypeInfo *args;
MemoryContext mcxt;
} PLyPlanObject;
extern void PLy_plan_init_type(void);
......
......@@ -112,8 +112,9 @@ PLy_procedure_get(Oid fn_oid, Oid fn_rel, bool is_trigger)
else if (!PLy_procedure_valid(proc, procTup))
{
/* Found it, but it's invalid, free and reuse the cache entry */
entry->proc = NULL;
if (proc)
PLy_procedure_delete(proc);
PLy_free(proc);
proc = PLy_procedure_create(procTup, fn_oid, is_trigger);
entry->proc = proc;
}
......@@ -142,11 +143,9 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
char procName[NAMEDATALEN + 256];
Form_pg_proc procStruct;
PLyProcedure *volatile proc;
char *volatile procSource = NULL;
Datum prosrcdatum;
bool isnull;
int i,
rv;
MemoryContext cxt;
MemoryContext oldcxt;
int rv;
procStruct = (Form_pg_proc) GETSTRUCT(procTup);
rv = snprintf(procName, sizeof(procName),
......@@ -156,29 +155,41 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
if (rv >= sizeof(procName) || rv < 0)
elog(ERROR, "procedure name would overrun buffer");
proc = PLy_malloc(sizeof(PLyProcedure));
proc->proname = PLy_strdup(NameStr(procStruct->proname));
proc->pyname = PLy_strdup(procName);
cxt = AllocSetContextCreate(TopMemoryContext,
procName,
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
oldcxt = MemoryContextSwitchTo(cxt);
proc = (PLyProcedure *) palloc0(sizeof(PLyProcedure));
proc->mcxt = cxt;
PG_TRY();
{
Datum protrftypes_datum;
Datum prosrcdatum;
bool isnull;
char *procSource;
int i;
proc->proname = pstrdup(NameStr(procStruct->proname));
proc->pyname = pstrdup(procName);
proc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
proc->fn_tid = procTup->t_self;
/* Remember if function is STABLE/IMMUTABLE */
proc->fn_readonly =
(procStruct->provolatile != PROVOLATILE_VOLATILE);
PLy_typeinfo_init(&proc->result);
PLy_typeinfo_init(&proc->result, proc->mcxt);
for (i = 0; i < FUNC_MAX_ARGS; i++)
PLy_typeinfo_init(&proc->args[i]);
PLy_typeinfo_init(&proc->args[i], proc->mcxt);
proc->nargs = 0;
proc->langid = procStruct->prolang;
{
MemoryContext oldcxt;
Datum protrftypes_datum = SysCacheGetAttr(PROCOID, procTup,
Anum_pg_proc_protrftypes, &isnull);
oldcxt = MemoryContextSwitchTo(TopMemoryContext);
protrftypes_datum = SysCacheGetAttr(PROCOID, procTup,
Anum_pg_proc_protrftypes,
&isnull);
proc->trftypes = isnull ? NIL : oid_array_to_list(protrftypes_datum);
MemoryContextSwitchTo(oldcxt);
}
proc->code = proc->statics = NULL;
proc->globals = NULL;
proc->is_setof = procStruct->proretset;
......@@ -186,8 +197,6 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
proc->src = NULL;
proc->argnames = NULL;
PG_TRY();
{
/*
* get information required for output conversion of the return value,
* but only if this isn't a trigger.
......@@ -250,8 +259,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
Oid *types;
char **names,
*modes;
int i,
pos,
int pos,
total;
/* extract argument type info from the pg_proc tuple */
......@@ -271,7 +279,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
}
}
proc->argnames = (char **) PLy_malloc0(sizeof(char *) * proc->nargs);
proc->argnames = (char **) palloc0(sizeof(char *) * proc->nargs);
for (i = pos = 0; i < total; i++)
{
HeapTuple argTypeTup;
......@@ -314,7 +322,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
}
/* get argument name */
proc->argnames[pos] = names ? PLy_strdup(names[i]) : NULL;
proc->argnames[pos] = names ? pstrdup(names[i]) : NULL;
ReleaseSysCache(argTypeTup);
......@@ -334,18 +342,16 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
PLy_procedure_compile(proc, procSource);
pfree(procSource);
procSource = NULL;
}
PG_CATCH();
{
MemoryContextSwitchTo(oldcxt);
PLy_procedure_delete(proc);
if (procSource)
pfree(procSource);
PG_RE_THROW();
}
PG_END_TRY();
MemoryContextSwitchTo(oldcxt);
return proc;
}
......@@ -372,7 +378,7 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src)
*/
msrc = PLy_procedure_munge_source(proc->pyname, src);
/* Save the mangled source for later inclusion in tracebacks */
proc->src = PLy_strdup(msrc);
proc->src = MemoryContextStrdup(proc->mcxt, msrc);
crv = PyRun_String(msrc, Py_file_input, proc->globals, NULL);
pfree(msrc);
......@@ -404,31 +410,10 @@ PLy_procedure_compile(PLyProcedure *proc, const char *src)
void
PLy_procedure_delete(PLyProcedure *proc)
{
int i;
Py_XDECREF(proc->code);
Py_XDECREF(proc->statics);
Py_XDECREF(proc->globals);
if (proc->proname)
PLy_free(proc->proname);
if (proc->pyname)
PLy_free(proc->pyname);
for (i = 0; i < proc->nargs; i++)
{
if (proc->args[i].is_rowtype == 1)
{
if (proc->args[i].in.r.atts)
PLy_free(proc->args[i].in.r.atts);
if (proc->args[i].out.r.atts)
PLy_free(proc->args[i].out.r.atts);
}
if (proc->argnames && proc->argnames[i])
PLy_free(proc->argnames[i]);
}
if (proc->src)
PLy_free(proc->src);
if (proc->argnames)
PLy_free(proc->argnames);
MemoryContextDelete(proc->mcxt);
}
/*
......@@ -479,7 +464,8 @@ PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
int i;
bool valid;
Assert(proc != NULL);
if (proc == NULL)
return false;
/* If the pg_proc tuple has changed, it's not valid */
if (!(proc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
......
......@@ -14,6 +14,8 @@ extern void init_procedure_caches(void);
/* cached procedure data */
typedef struct PLyProcedure
{
MemoryContext mcxt; /* context holding this PLyProcedure and its
* subsidiary data */
char *proname; /* SQL name of procedure */
char *pyname; /* Python name of procedure */
TransactionId fn_xmin;
......
......@@ -61,12 +61,21 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
if ((plan = (PLyPlanObject *) PLy_plan_new()) == NULL)
return NULL;
plan->mcxt = AllocSetContextCreate(TopMemoryContext,
"PL/Python plan context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
oldcontext = MemoryContextSwitchTo(plan->mcxt);
nargs = list ? PySequence_Length(list) : 0;
plan->nargs = nargs;
plan->types = nargs ? PLy_malloc(sizeof(Oid) * nargs) : NULL;
plan->values = nargs ? PLy_malloc(sizeof(Datum) * nargs) : NULL;
plan->args = nargs ? PLy_malloc(sizeof(PLyTypeInfo) * nargs) : NULL;
plan->types = nargs ? palloc(sizeof(Oid) * nargs) : NULL;
plan->values = nargs ? palloc(sizeof(Datum) * nargs) : NULL;
plan->args = nargs ? palloc(sizeof(PLyTypeInfo) * nargs) : NULL;
MemoryContextSwitchTo(oldcontext);
oldcontext = CurrentMemoryContext;
oldowner = CurrentResourceOwner;
......@@ -84,7 +93,7 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
*/
for (i = 0; i < nargs; i++)
{
PLy_typeinfo_init(&plan->args[i]);
PLy_typeinfo_init(&plan->args[i], plan->mcxt);
plan->values[i] = PointerGetDatum(NULL);
}
......@@ -391,10 +400,17 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
{
PLyTypeInfo args;
int i;
MemoryContext cxt;
Py_DECREF(result->nrows);
result->nrows = PyInt_FromLong(rows);
PLy_typeinfo_init(&args);
cxt = AllocSetContextCreate(CurrentMemoryContext,
"PL/Python temp context",
ALLOCSET_DEFAULT_MINSIZE,
ALLOCSET_DEFAULT_INITSIZE,
ALLOCSET_DEFAULT_MAXSIZE);
PLy_typeinfo_init(&args, cxt);
oldcontext = CurrentMemoryContext;
PG_TRY();
......@@ -432,13 +448,13 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
PG_CATCH();
{
MemoryContextSwitchTo(oldcontext);
PLy_typeinfo_dealloc(&args);
MemoryContextDelete(cxt);
Py_DECREF(result);
PG_RE_THROW();
}
PG_END_TRY();
PLy_typeinfo_dealloc(&args);
MemoryContextDelete(cxt);
SPI_freetuptable(tuptable);
}
......
......@@ -8,6 +8,7 @@
#include "access/xact.h"
#include "executor/spi.h"
#include "utils/memutils.h"
#include "plpython.h"
......@@ -132,16 +133,22 @@ PLy_subtransaction_enter(PyObject *self, PyObject *unused)
subxact->started = true;
oldcontext = CurrentMemoryContext;
subxactdata = PLy_malloc(sizeof(*subxactdata));
subxactdata = (PLySubtransactionData *)
MemoryContextAlloc(TopTransactionContext,
sizeof(PLySubtransactionData));
subxactdata->oldcontext = oldcontext;
subxactdata->oldowner = CurrentResourceOwner;
BeginInternalSubTransaction(NULL);
/* Do not want to leave the previous memory context */
MemoryContextSwitchTo(oldcontext);
/* Be sure that cells of explicit_subtransactions list are long-lived */
MemoryContextSwitchTo(TopTransactionContext);
explicit_subtransactions = lcons(subxactdata, explicit_subtransactions);
/* Caller wants to stay in original memory context */
MemoryContextSwitchTo(oldcontext);
Py_INCREF(self);
return self;
}
......@@ -204,7 +211,7 @@ PLy_subtransaction_exit(PyObject *self, PyObject *args)
MemoryContextSwitchTo(subxactdata->oldcontext);
CurrentResourceOwner = subxactdata->oldowner;
PLy_free(subxactdata);
pfree(subxactdata);
/*
* AtEOSubXact_SPI() should not have popped any SPI context, but just in
......
This diff is collapsed.
......@@ -88,10 +88,12 @@ typedef struct PLyTypeInfo
Oid typ_relid;
TransactionId typrel_xmin;
ItemPointerData typrel_tid;
/* context for subsidiary data (doesn't belong to this struct though) */
MemoryContext mcxt;
} PLyTypeInfo;
extern void PLy_typeinfo_init(PLyTypeInfo *arg);
extern void PLy_typeinfo_dealloc(PLyTypeInfo *arg);
extern void PLy_typeinfo_init(PLyTypeInfo *arg, MemoryContext mcxt);
extern void PLy_input_datum_func(PLyTypeInfo *arg, Oid typeOid, HeapTuple typeTup, Oid langid, List *trftypes);
extern void PLy_output_datum_func(PLyTypeInfo *arg, HeapTuple typeTup, Oid langid, List *trftypes);
......
......@@ -17,42 +17,6 @@
#include "plpy_elog.h"
void *
PLy_malloc(size_t bytes)
{
/* We need our allocations to be long-lived, so use TopMemoryContext */
return MemoryContextAlloc(TopMemoryContext, bytes);
}
void *
PLy_malloc0(size_t bytes)
{
void *ptr = PLy_malloc(bytes);
MemSet(ptr, 0, bytes);
return ptr;
}
char *
PLy_strdup(const char *str)
{
char *result;
size_t len;
len = strlen(str) + 1;
result = PLy_malloc(len);
memcpy(result, str, len);
return result;
}
/* define this away */
void
PLy_free(void *ptr)
{
pfree(ptr);
}
/*
* Convert a Python unicode object to a Python string/bytes object in
* PostgreSQL server encoding. Reference ownership is passed to the
......
......@@ -6,11 +6,6 @@
#ifndef PLPY_UTIL_H
#define PLPY_UTIL_H
extern void *PLy_malloc(size_t bytes);
extern void *PLy_malloc0(size_t bytes);
extern char *PLy_strdup(const char *str);
extern void PLy_free(void *ptr);
extern PyObject *PLyUnicode_Bytes(PyObject *unicode);
extern char *PLyUnicode_AsString(PyObject *unicode);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment