Skip to content

Commit 29dfffa

Browse files
tglsfdctglsfdc
committed
Repair memory leaks in plpython.
PLy_spi_execute_plan (PLyPlan.execute) and PLy_cursor_plan (plpy.cursor) use PLy_output_convert to convert Python values into Datums that can be passed to the query-to-execute. But they failed to pay much attention to its warning that it can leave "cruft generated along the way" behind. Repeated use of these methods can result in a substantial memory leak for the duration of the calling plpython function. To fix, make a temporary memory context to invoke PLy_output_convert in. This also lets us get rid of the rather fragile code that was here for retail pfree's of the converted Datums. Indeed, we don't need the PLyPlanObject.values field anymore at all, though I left it in place in the back branches in the name of ABI stability. Mat Arye and Tom Lane, per report from Mat Arye. Back-patch to all supported branches. Discussion: https://postgr.es/m/CADsUR0DvVgnZYWwnmKRK65MZg7YLUSTDLV61qdnrwtrAJgU6xw@mail.gmail.com
1 parent ca87c41 commit 29dfffa

File tree

4 files changed

+49
-64
lines changed

4 files changed

+49
-64
lines changed

‎src/pl/plpython/plpy_cursorobject.c

+24-28
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
140140
{
141141
PLyCursorObject *cursor;
142142
volatile int nargs;
143-
int i;
144143
PLyPlanObject *plan;
145144
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
146145
volatile MemoryContext oldcontext;
@@ -199,13 +198,30 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
199198
PG_TRY();
200199
{
201200
Portal portal;
201+
MemoryContext tmpcontext;
202+
Datum *volatile values;
202203
char *volatile nulls;
203204
volatile int j;
204205

206+
/*
207+
* Converted arguments and associated cruft will be in this context,
208+
* which is local to our subtransaction.
209+
*/
210+
tmpcontext = AllocSetContextCreate(CurTransactionContext,
211+
"PL/Python temporary context",
212+
ALLOCSET_SMALL_SIZES);
213+
MemoryContextSwitchTo(tmpcontext);
214+
205215
if (nargs > 0)
206-
nulls = palloc(nargs * sizeof(char));
216+
{
217+
values = (Datum *) palloc(nargs * sizeof(Datum));
218+
nulls = (char *) palloc(nargs * sizeof(char));
219+
}
207220
else
221+
{
222+
values = NULL;
208223
nulls = NULL;
224+
}
209225

210226
for (j = 0; j < nargs; j++)
211227
{
@@ -217,7 +233,7 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
217233
{
218234
bool isnull;
219235

220-
plan->values[j] = PLy_output_convert(arg, elem, &isnull);
236+
values[j] = PLy_output_convert(arg, elem, &isnull);
221237
nulls[j] = isnull ? 'n' : ' ';
222238
}
223239
PG_FINALLY(2);
@@ -227,7 +243,9 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
227243
PG_END_TRY(2);
228244
}
229245

230-
portal = SPI_cursor_open(NULL, plan->plan, plan->values, nulls,
246+
MemoryContextSwitchTo(oldcontext);
247+
248+
portal = SPI_cursor_open(NULL, plan->plan, values, nulls,
231249
exec_ctx->curr_proc->fn_readonly);
232250
if (portal == NULL)
233251
elog(ERROR, "SPI_cursor_open() failed: %s",
@@ -237,40 +255,18 @@ PLy_cursor_plan(PyObject *ob, PyObject *args)
237255

238256
PinPortal(portal);
239257

258+
MemoryContextDelete(tmpcontext);
240259
PLy_spi_subtransaction_commit(oldcontext, oldowner);
241260
}
242261
PG_CATCH();
243262
{
244-
int k;
245-
246-
/* cleanup plan->values array */
247-
for (k = 0; k < nargs; k++)
248-
{
249-
if (!plan->args[k].typbyval &&
250-
(plan->values[k] != PointerGetDatum(NULL)))
251-
{
252-
pfree(DatumGetPointer(plan->values[k]));
253-
plan->values[k] = PointerGetDatum(NULL);
254-
}
255-
}
256-
257263
Py_DECREF(cursor);
258-
264+
/* Subtransaction abort will remove the tmpcontext */
259265
PLy_spi_subtransaction_abort(oldcontext, oldowner);
260266
return NULL;
261267
}
262268
PG_END_TRY();
263269

264-
for (i = 0; i < nargs; i++)
265-
{
266-
if (!plan->args[i].typbyval &&
267-
(plan->values[i] != PointerGetDatum(NULL)))
268-
{
269-
pfree(DatumGetPointer(plan->values[i]));
270-
plan->values[i] = PointerGetDatum(NULL);
271-
}
272-
}
273-
274270
Assert(cursor->portalname != NULL);
275271
return (PyObject *) cursor;
276272
}

‎src/pl/plpython/plpy_planobject.c

-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ PLy_plan_new(void)
5454
ob->plan = NULL;
5555
ob->nargs = 0;
5656
ob->types = NULL;
57-
ob->values = NULL;
5857
ob->args = NULL;
5958
ob->mcxt = NULL;
6059

‎src/pl/plpython/plpy_planobject.h

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ typedef struct PLyPlanObject
1515
SPIPlanPtr plan;
1616
int nargs;
1717
Oid *types;
18-
Datum *values;
1918
PLyObToDatum *args;
2019
MemoryContext mcxt;
2120
} PLyPlanObject;

‎src/pl/plpython/plpy_spi.c

+25-34
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
6666

6767
plan->nargs = nargs;
6868
plan->types = nargs ? palloc0(sizeof(Oid) * nargs) : NULL;
69-
plan->values = nargs ? palloc0(sizeof(Datum) * nargs) : NULL;
7069
plan->args = nargs ? palloc0(sizeof(PLyObToDatum) * nargs) : NULL;
7170

7271
MemoryContextSwitchTo(oldcontext);
@@ -172,8 +171,7 @@ PyObject *
172171
PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
173172
{
174173
volatile int nargs;
175-
int i,
176-
rv;
174+
int rv;
177175
PLyPlanObject *plan;
178176
volatile MemoryContext oldcontext;
179177
volatile ResourceOwner oldowner;
@@ -219,13 +217,30 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
219217
PG_TRY();
220218
{
221219
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
220+
MemoryContext tmpcontext;
221+
Datum *volatile values;
222222
char *volatile nulls;
223223
volatile int j;
224224

225+
/*
226+
* Converted arguments and associated cruft will be in this context,
227+
* which is local to our subtransaction.
228+
*/
229+
tmpcontext = AllocSetContextCreate(CurTransactionContext,
230+
"PL/Python temporary context",
231+
ALLOCSET_SMALL_SIZES);
232+
MemoryContextSwitchTo(tmpcontext);
233+
225234
if (nargs > 0)
226-
nulls = palloc(nargs * sizeof(char));
235+
{
236+
values = (Datum *) palloc(nargs * sizeof(Datum));
237+
nulls = (char *) palloc(nargs * sizeof(char));
238+
}
227239
else
240+
{
241+
values = NULL;
228242
nulls = NULL;
243+
}
229244

230245
for (j = 0; j < nargs; j++)
231246
{
@@ -237,7 +252,7 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
237252
{
238253
bool isnull;
239254

240-
plan->values[j] = PLy_output_convert(arg, elem, &isnull);
255+
values[j] = PLy_output_convert(arg, elem, &isnull);
241256
nulls[j] = isnull ? 'n' : ' ';
242257
}
243258
PG_FINALLY(2);
@@ -247,47 +262,23 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
247262
PG_END_TRY(2);
248263
}
249264

250-
rv = SPI_execute_plan(plan->plan, plan->values, nulls,
265+
MemoryContextSwitchTo(oldcontext);
266+
267+
rv = SPI_execute_plan(plan->plan, values, nulls,
251268
exec_ctx->curr_proc->fn_readonly, limit);
252269
ret = PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
253270

254-
if (nargs > 0)
255-
pfree(nulls);
256-
271+
MemoryContextDelete(tmpcontext);
257272
PLy_spi_subtransaction_commit(oldcontext, oldowner);
258273
}
259274
PG_CATCH();
260275
{
261-
int k;
262-
263-
/*
264-
* cleanup plan->values array
265-
*/
266-
for (k = 0; k < nargs; k++)
267-
{
268-
if (!plan->args[k].typbyval &&
269-
(plan->values[k] != PointerGetDatum(NULL)))
270-
{
271-
pfree(DatumGetPointer(plan->values[k]));
272-
plan->values[k] = PointerGetDatum(NULL);
273-
}
274-
}
275-
276+
/* Subtransaction abort will remove the tmpcontext */
276277
PLy_spi_subtransaction_abort(oldcontext, oldowner);
277278
return NULL;
278279
}
279280
PG_END_TRY();
280281

281-
for (i = 0; i < nargs; i++)
282-
{
283-
if (!plan->args[i].typbyval &&
284-
(plan->values[i] != PointerGetDatum(NULL)))
285-
{
286-
pfree(DatumGetPointer(plan->values[i]));
287-
plan->values[i] = PointerGetDatum(NULL);
288-
}
289-
}
290-
291282
if (rv < 0)
292283
{
293284
PLy_exception_set(PLy_exc_spi_error,

0 commit comments

Comments
 (0)