From ac48cc9ca7ceb8b1ed0e51a3902b8879673fbf73 Mon Sep 17 00:00:00 2001 From: Alexey Grishchenko Date: Thu, 10 Mar 2016 12:56:06 +0000 Subject: [PATCH] Fix endless loop in plpython set-returning function The issue occurs when the same set-returning function is called twice within a single query. The issue is caused by the fact that result iterator is stored in the procedure data structure, which is reused for different calls of the same procedure. But in fact, when you execute the same SRF twice in the same query, the result iterator is being continuously reinitialized by two different instances of the same function, the execution never ends and process dies with OOM Also this fix changes the PLy_function_delete_args, because with calling two SRF in a single query the old implementation would attempt to delete the same input parameter twice, which would lead to key error Regression test added for this case --- src/pl/plpython/expected/plpython_setof.out | 12 ++++++++ src/pl/plpython/plpy_exec.c | 45 ++++++++++++++++++++++------- src/pl/plpython/plpy_procedure.c | 1 - src/pl/plpython/plpy_procedure.h | 1 - src/pl/plpython/sql/plpython_setof.sql | 3 ++ 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/pl/plpython/expected/plpython_setof.out b/src/pl/plpython/expected/plpython_setof.out index 62b8a45..d35f9c2 100644 --- a/src/pl/plpython/expected/plpython_setof.out +++ b/src/pl/plpython/expected/plpython_setof.out @@ -124,6 +124,18 @@ SELECT test_setof_spi_in_iterator(); World (4 rows) +-- Calling the same set-returning function twice in a single query +select test_setof_as_list(2, 'list'), test_setof_as_list(3, 'list'); + test_setof_as_list | test_setof_as_list +--------------------+-------------------- + list | list + list | list + list | list + list | list + list | list + list | list +(6 rows) + -- returns set of named-composite-type tuples CREATE OR REPLACE FUNCTION get_user_records() RETURNS SETOF users diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c index 24aed01..f68f310 100644 --- a/src/pl/plpython/plpy_exec.c +++ b/src/pl/plpython/plpy_exec.c @@ -47,11 +47,22 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) Datum rv; PyObject *volatile plargs = NULL; PyObject *volatile plrv = NULL; + FuncCallContext *volatile funcctx = NULL; ErrorContextCallback plerrcontext; PG_TRY(); { - if (!proc->is_setof || proc->setof == NULL) + if (proc->is_setof) + { + /* First Call setup */ + if (SRF_IS_FIRSTCALL()) + funcctx = SRF_FIRSTCALL_INIT(); + /* Every call setup */ + funcctx = SRF_PERCALL_SETUP(); + Assert(funcctx != NULL); + } + + if (!proc->is_setof || funcctx->user_fctx == NULL) { /* * Simple type returning function or first time for SETOF @@ -80,7 +91,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) bool has_error = false; ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo; - if (proc->setof == NULL) + if (funcctx->user_fctx == NULL) { /* first time -- do checks and setup */ if (!rsi || !IsA(rsi, ReturnSetInfo) || @@ -94,11 +105,11 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) rsi->returnMode = SFRM_ValuePerCall; /* Make iterator out of returned object */ - proc->setof = PyObject_GetIter(plrv); + funcctx->user_fctx = (void*) PyObject_GetIter(plrv); Py_DECREF(plrv); plrv = NULL; - if (proc->setof == NULL) + if (funcctx->user_fctx == NULL) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("returned object cannot be iterated"), @@ -106,7 +117,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) } /* Fetch next from iterator */ - plrv = PyIter_Next(proc->setof); + plrv = PyIter_Next(funcctx->user_fctx); if (plrv) rsi->isDone = ExprMultipleResult; else @@ -118,8 +129,8 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) if (rsi->isDone == ExprEndResult) { /* Iterator is exhausted or error happened */ - Py_DECREF(proc->setof); - proc->setof = NULL; + Py_DECREF( (PyObject*) funcctx->user_fctx); + funcctx->user_fctx = NULL; Py_XDECREF(plargs); Py_XDECREF(plrv); @@ -134,7 +145,7 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) elog(ERROR, "SPI_finish failed"); fcinfo->isnull = true; - return (Datum) NULL; + SRF_RETURN_DONE(funcctx); } } @@ -213,8 +224,11 @@ PLy_exec_function(FunctionCallInfo fcinfo, PLyProcedure *proc) * yet. Set it to NULL so the next invocation of the function will * start the iteration again. */ - Py_XDECREF(proc->setof); - proc->setof = NULL; + if (proc->is_setof && funcctx->user_fctx != NULL) + { + Py_XDECREF( (PyObject*) funcctx->user_fctx ); + funcctx->user_fctx = NULL; + } PG_RE_THROW(); } @@ -435,13 +449,22 @@ static void PLy_function_delete_args(PLyProcedure *proc) { int i; + PyObject *arg; if (!proc->argnames) return; for (i = 0; i < proc->nargs; i++) if (proc->argnames[i]) - PyDict_DelItemString(proc->globals, proc->argnames[i]); + { + arg = PyString_FromString(proc->argnames[i]); + + /* Deleting the item only if it exists in the dictionaty */ + if (PyDict_Contains(proc->globals, arg)) + PyDict_DelItem(proc->globals, arg); + + Py_DECREF(arg); + } } static void diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c index a0d0792..5cad06d 100644 --- a/src/pl/plpython/plpy_procedure.c +++ b/src/pl/plpython/plpy_procedure.c @@ -203,7 +203,6 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger) proc->code = proc->statics = NULL; proc->globals = NULL; proc->is_setof = procStruct->proretset; - proc->setof = NULL; proc->src = NULL; proc->argnames = NULL; diff --git a/src/pl/plpython/plpy_procedure.h b/src/pl/plpython/plpy_procedure.h index 9fc8db0..2ec10d3 100644 --- a/src/pl/plpython/plpy_procedure.h +++ b/src/pl/plpython/plpy_procedure.h @@ -24,7 +24,6 @@ typedef struct PLyProcedure PLyTypeInfo result; /* also used to store info for trigger tuple * type */ bool is_setof; /* true, if procedure returns result set */ - PyObject *setof; /* contents of result set. */ char *src; /* textual procedure code, after mangling */ char **argnames; /* Argument names */ PLyTypeInfo args[FUNC_MAX_ARGS]; diff --git a/src/pl/plpython/sql/plpython_setof.sql b/src/pl/plpython/sql/plpython_setof.sql index fe034fb..63ca466 100644 --- a/src/pl/plpython/sql/plpython_setof.sql +++ b/src/pl/plpython/sql/plpython_setof.sql @@ -63,6 +63,9 @@ SELECT test_setof_as_iterator(2, null); SELECT test_setof_spi_in_iterator(); +-- Calling the same set-returning function twice in a single query +select test_setof_as_list(2, 'list'), test_setof_as_list(3, 'list'); + -- returns set of named-composite-type tuples CREATE OR REPLACE FUNCTION get_user_records() -- 1.9.5 (Apple Git-50.3)