1 files changed, 175 insertions, 170 deletions

diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
index ddf3573f0e7..f6d10045e5c 100644
--- a/src/pl/plpython/plpy_elog.c
+++ b/src/pl/plpython/plpy_elog.c
@@ -18,7 +18,8 @@ PyObject   *PLy_exc_spi_error = NULL;
 
 
 static void PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
-						  char **xmsg, char **tbmsg, int *tb_depth);
+						  char *volatile *xmsg, char *volatile *tbmsg,
+						  int *tb_depth);
 static void PLy_get_spi_error_data(PyObject *exc, int *sqlerrcode, char **detail,
 								   char **hint, char **query, int *position,
 								   char **schema_name, char **table_name, char **column_name,
@@ -43,78 +44,82 @@ void
 PLy_elog_impl(int elevel, const char *fmt,...)
 {
 	int			save_errno = errno;
-	char	   *xmsg;
-	char	   *tbmsg;
+	char	   *volatile xmsg = NULL;
+	char	   *volatile tbmsg = NULL;
 	int			tb_depth;
 	StringInfoData emsg;
 	PyObject   *exc,
 			   *val,
 			   *tb;
-	const char *primary = NULL;
-	int			sqlerrcode = 0;
-	char	   *detail = NULL;
-	char	   *hint = NULL;
-	char	   *query = NULL;
-	int			position = 0;
-	char	   *schema_name = NULL;
-	char	   *table_name = NULL;
-	char	   *column_name = NULL;
-	char	   *datatype_name = NULL;
-	char	   *constraint_name = NULL;
+
+	/* If we'll need emsg, must initialize it before entering PG_TRY */
+	if (fmt)
+		initStringInfo(&emsg);
 
 	PyErr_Fetch(&exc, &val, &tb);
 
-	if (exc != NULL)
+	/* Use a PG_TRY block to ensure we release the PyObjects just acquired */
+	PG_TRY();
 	{
-		PyErr_NormalizeException(&exc, &val, &tb);
-
-		if (PyErr_GivenExceptionMatches(val, PLy_exc_spi_error))
-			PLy_get_spi_error_data(val, &sqlerrcode,
-								   &detail, &hint, &query, &position,
+		const char *primary = NULL;
+		int			sqlerrcode = 0;
+		char	   *detail = NULL;
+		char	   *hint = NULL;
+		char	   *query = NULL;
+		int			position = 0;
+		char	   *schema_name = NULL;
+		char	   *table_name = NULL;
+		char	   *column_name = NULL;
+		char	   *datatype_name = NULL;
+		char	   *constraint_name = NULL;
+
+		if (exc != NULL)
+		{
+			PyErr_NormalizeException(&exc, &val, &tb);
+
+			if (PyErr_GivenExceptionMatches(val, PLy_exc_spi_error))
+				PLy_get_spi_error_data(val, &sqlerrcode,
+									   &detail, &hint, &query, &position,
+									   &schema_name, &table_name, &column_name,
+									   &datatype_name, &constraint_name);
+			else if (PyErr_GivenExceptionMatches(val, PLy_exc_error))
+				PLy_get_error_data(val, &sqlerrcode, &detail, &hint,
 								   &schema_name, &table_name, &column_name,
 								   &datatype_name, &constraint_name);
-		else if (PyErr_GivenExceptionMatches(val, PLy_exc_error))
-			PLy_get_error_data(val, &sqlerrcode, &detail, &hint,
-							   &schema_name, &table_name, &column_name,
-							   &datatype_name, &constraint_name);
-		else if (PyErr_GivenExceptionMatches(val, PLy_exc_fatal))
-			elevel = FATAL;
-	}
+			else if (PyErr_GivenExceptionMatches(val, PLy_exc_fatal))
+				elevel = FATAL;
+		}
 
-	/* this releases our refcount on tb! */
-	PLy_traceback(exc, val, tb,
-				  &xmsg, &tbmsg, &tb_depth);
+		PLy_traceback(exc, val, tb,
+					  &xmsg, &tbmsg, &tb_depth);
 
-	if (fmt)
-	{
-		initStringInfo(&emsg);
-		for (;;)
+		if (fmt)
 		{
-			va_list		ap;
-			int			needed;
-
-			errno = save_errno;
-			va_start(ap, fmt);
-			needed = appendStringInfoVA(&emsg, dgettext(TEXTDOMAIN, fmt), ap);
-			va_end(ap);
-			if (needed == 0)
-				break;
-			enlargeStringInfo(&emsg, needed);
-		}
-		primary = emsg.data;
+			for (;;)
+			{
+				va_list		ap;
+				int			needed;
+
+				errno = save_errno;
+				va_start(ap, fmt);
+				needed = appendStringInfoVA(&emsg, dgettext(TEXTDOMAIN, fmt), ap);
+				va_end(ap);
+				if (needed == 0)
+					break;
+				enlargeStringInfo(&emsg, needed);
+			}
+			primary = emsg.data;
 
-		/* If there's an exception message, it goes in the detail. */
-		if (xmsg)
-			detail = xmsg;
-	}
-	else
-	{
-		if (xmsg)
-			primary = xmsg;
-	}
+			/* If there's an exception message, it goes in the detail. */
+			if (xmsg)
+				detail = xmsg;
+		}
+		else
+		{
+			if (xmsg)
+				primary = xmsg;
+		}
 
-	PG_TRY();
-	{
 		ereport(elevel,
 				(errcode(sqlerrcode ? sqlerrcode : ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
 				 errmsg_internal("%s", primary ? primary : "no exception data"),
@@ -136,14 +141,23 @@ PLy_elog_impl(int elevel, const char *fmt,...)
 	}
 	PG_FINALLY();
 	{
+		Py_XDECREF(exc);
+		Py_XDECREF(val);
+		/* Must release all the objects in the traceback stack */
+		while (tb != NULL && tb != Py_None)
+		{
+			PyObject   *tb_prev = tb;
+
+			tb = PyObject_GetAttrString(tb, "tb_next");
+			Py_DECREF(tb_prev);
+		}
+		/* For neatness' sake, also release our string buffers */
 		if (fmt)
 			pfree(emsg.data);
 		if (xmsg)
 			pfree(xmsg);
 		if (tbmsg)
 			pfree(tbmsg);
-		Py_XDECREF(exc);
-		Py_XDECREF(val);
 	}
 	PG_END_TRY();
 }
@@ -154,21 +168,14 @@ PLy_elog_impl(int elevel, const char *fmt,...)
  * The exception error message is returned in xmsg, the traceback in
  * tbmsg (both as palloc'd strings) and the traceback depth in
  * tb_depth.
- *
- * We release refcounts on all the Python objects in the traceback stack,
- * but not on e or v.
  */
 static void
 PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
-			  char **xmsg, char **tbmsg, int *tb_depth)
+			  char *volatile *xmsg, char *volatile *tbmsg, int *tb_depth)
 {
-	PyObject   *e_type_o;
-	PyObject   *e_module_o;
-	char	   *e_type_s = NULL;
-	char	   *e_module_s = NULL;
-	PyObject   *vob = NULL;
-	char	   *vstr;
-	StringInfoData xstr;
+	PyObject   *volatile e_type_o = NULL;
+	PyObject   *volatile e_module_o = NULL;
+	PyObject   *volatile vob = NULL;
 	StringInfoData tbstr;
 
 	/*
@@ -186,47 +193,59 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
 	/*
 	 * Format the exception and its value and put it in xmsg.
 	 */
-
-	e_type_o = PyObject_GetAttrString(e, "__name__");
-	e_module_o = PyObject_GetAttrString(e, "__module__");
-	if (e_type_o)
-		e_type_s = PLyUnicode_AsString(e_type_o);
-	if (e_type_s)
-		e_module_s = PLyUnicode_AsString(e_module_o);
-
-	if (v && ((vob = PyObject_Str(v)) != NULL))
-		vstr = PLyUnicode_AsString(vob);
-	else
-		vstr = "unknown";
-
-	initStringInfo(&xstr);
-	if (!e_type_s || !e_module_s)
+	PG_TRY();
 	{
-		/* shouldn't happen */
-		appendStringInfoString(&xstr, "unrecognized exception");
+		char	   *e_type_s = NULL;
+		char	   *e_module_s = NULL;
+		const char *vstr;
+		StringInfoData xstr;
+
+		e_type_o = PyObject_GetAttrString(e, "__name__");
+		e_module_o = PyObject_GetAttrString(e, "__module__");
+		if (e_type_o)
+			e_type_s = PLyUnicode_AsString(e_type_o);
+		if (e_module_o)
+			e_module_s = PLyUnicode_AsString(e_module_o);
+
+		if (v && ((vob = PyObject_Str(v)) != NULL))
+			vstr = PLyUnicode_AsString(vob);
+		else
+			vstr = "unknown";
+
+		initStringInfo(&xstr);
+		if (!e_type_s || !e_module_s)
+		{
+			/* shouldn't happen */
+			appendStringInfoString(&xstr, "unrecognized exception");
+		}
+		/* mimics behavior of traceback.format_exception_only */
+		else if (strcmp(e_module_s, "builtins") == 0
+				 || strcmp(e_module_s, "__main__") == 0
+				 || strcmp(e_module_s, "exceptions") == 0)
+			appendStringInfoString(&xstr, e_type_s);
+		else
+			appendStringInfo(&xstr, "%s.%s", e_module_s, e_type_s);
+		appendStringInfo(&xstr, ": %s", vstr);
+
+		*xmsg = xstr.data;
 	}
-	/* mimics behavior of traceback.format_exception_only */
-	else if (strcmp(e_module_s, "builtins") == 0
-			 || strcmp(e_module_s, "__main__") == 0
-			 || strcmp(e_module_s, "exceptions") == 0)
-		appendStringInfoString(&xstr, e_type_s);
-	else
-		appendStringInfo(&xstr, "%s.%s", e_module_s, e_type_s);
-	appendStringInfo(&xstr, ": %s", vstr);
-
-	*xmsg = xstr.data;
+	PG_FINALLY();
+	{
+		Py_XDECREF(e_type_o);
+		Py_XDECREF(e_module_o);
+		Py_XDECREF(vob);
+	}
+	PG_END_TRY();
 
 	/*
 	 * Now format the traceback and put it in tbmsg.
 	 */
-
 	*tb_depth = 0;
 	initStringInfo(&tbstr);
 	/* Mimic Python traceback reporting as close as possible. */
 	appendStringInfoString(&tbstr, "Traceback (most recent call last):");
 	while (tb != NULL && tb != Py_None)
 	{
-		PyObject   *volatile tb_prev = NULL;
 		PyObject   *volatile frame = NULL;
 		PyObject   *volatile code = NULL;
 		PyObject   *volatile name = NULL;
@@ -254,84 +273,74 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
 			filename = PyObject_GetAttrString(code, "co_filename");
 			if (filename == NULL)
 				elog(ERROR, "could not get file name from Python code object");
+
+			/* The first frame always points at <module>, skip it. */
+			if (*tb_depth > 0)
+			{
+				PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+				char	   *proname;
+				char	   *fname;
+				char	   *line;
+				char	   *plain_filename;
+				long		plain_lineno;
+
+				/*
+				 * The second frame points at the internal function, but to
+				 * mimic Python error reporting we want to say <module>.
+				 */
+				if (*tb_depth == 1)
+					fname = "<module>";
+				else
+					fname = PLyUnicode_AsString(name);
+
+				proname = PLy_procedure_name(exec_ctx->curr_proc);
+				plain_filename = PLyUnicode_AsString(filename);
+				plain_lineno = PyLong_AsLong(lineno);
+
+				if (proname == NULL)
+					appendStringInfo(&tbstr, "\n  PL/Python anonymous code block, line %ld, in %s",
+									 plain_lineno - 1, fname);
+				else
+					appendStringInfo(&tbstr, "\n  PL/Python function \"%s\", line %ld, in %s",
+									 proname, plain_lineno - 1, fname);
+
+				/*
+				 * function code object was compiled with "<string>" as the
+				 * filename
+				 */
+				if (exec_ctx->curr_proc && plain_filename != NULL &&
+					strcmp(plain_filename, "<string>") == 0)
+				{
+					/*
+					 * If we know the current procedure, append the exact line
+					 * from the source, again mimicking Python's traceback.py
+					 * module behavior.  We could store the already line-split
+					 * source to avoid splitting it every time, but producing
+					 * a traceback is not the most important scenario to
+					 * optimize for.  But we do not go as far as traceback.py
+					 * in reading the source of imported modules.
+					 */
+					line = get_source_line(exec_ctx->curr_proc->src, plain_lineno);
+					if (line)
+					{
+						appendStringInfo(&tbstr, "\n    %s", line);
+						pfree(line);
+					}
+				}
+			}
 		}
-		PG_CATCH();
+		PG_FINALLY();
 		{
 			Py_XDECREF(frame);
 			Py_XDECREF(code);
 			Py_XDECREF(name);
 			Py_XDECREF(lineno);
 			Py_XDECREF(filename);
-			PG_RE_THROW();
 		}
 		PG_END_TRY();
 
-		/* The first frame always points at <module>, skip it. */
-		if (*tb_depth > 0)
-		{
-			PLyExecutionContext *exec_ctx = PLy_current_execution_context();
-			char	   *proname;
-			char	   *fname;
-			char	   *line;
-			char	   *plain_filename;
-			long		plain_lineno;
-
-			/*
-			 * The second frame points at the internal function, but to mimic
-			 * Python error reporting we want to say <module>.
-			 */
-			if (*tb_depth == 1)
-				fname = "<module>";
-			else
-				fname = PLyUnicode_AsString(name);
-
-			proname = PLy_procedure_name(exec_ctx->curr_proc);
-			plain_filename = PLyUnicode_AsString(filename);
-			plain_lineno = PyLong_AsLong(lineno);
-
-			if (proname == NULL)
-				appendStringInfo(&tbstr, "\n  PL/Python anonymous code block, line %ld, in %s",
-								 plain_lineno - 1, fname);
-			else
-				appendStringInfo(&tbstr, "\n  PL/Python function \"%s\", line %ld, in %s",
-								 proname, plain_lineno - 1, fname);
-
-			/*
-			 * function code object was compiled with "<string>" as the
-			 * filename
-			 */
-			if (exec_ctx->curr_proc && plain_filename != NULL &&
-				strcmp(plain_filename, "<string>") == 0)
-			{
-				/*
-				 * If we know the current procedure, append the exact line
-				 * from the source, again mimicking Python's traceback.py
-				 * module behavior.  We could store the already line-split
-				 * source to avoid splitting it every time, but producing a
-				 * traceback is not the most important scenario to optimize
-				 * for.  But we do not go as far as traceback.py in reading
-				 * the source of imported modules.
-				 */
-				line = get_source_line(exec_ctx->curr_proc->src, plain_lineno);
-				if (line)
-				{
-					appendStringInfo(&tbstr, "\n    %s", line);
-					pfree(line);
-				}
-			}
-		}
-
-		Py_DECREF(frame);
-		Py_DECREF(code);
-		Py_DECREF(name);
-		Py_DECREF(lineno);
-		Py_DECREF(filename);
-
-		/* Release the current frame and go to the next one. */
-		tb_prev = tb;
+		/* Advance to the next frame. */
 		tb = PyObject_GetAttrString(tb, "tb_next");
-		Assert(tb_prev != Py_None);
-		Py_DECREF(tb_prev);
 		if (tb == NULL)
 			elog(ERROR, "could not traverse Python traceback");
 		(*tb_depth)++;
@@ -339,10 +348,6 @@ PLy_traceback(PyObject *e, PyObject *v, PyObject *tb,
 
 	/* Return the traceback. */
 	*tbmsg = tbstr.data;
-
-	Py_XDECREF(e_type_o);
-	Py_XDECREF(e_module_o);
-	Py_XDECREF(vob);
 }
 
 /*