Hello, hackers.
I was investigating Valgrind issues with plpython. It turns out
python itself doesn't play well with Valgrind in default build.
Therefore I built python with valgrind related flags
--with-valgrind --without-pymalloc
and added debug flags just to be sure
--with-pydebug --with-assertions
It causes plpython's tests to fail on internal python's
assertions.
Example backtrace (python version 3.7, postgresql master branch):
#8 0x00007fbf02851662 in __GI___assert_fail "!PyErr_Occurred()"
at assert.c:101
#9 0x00007fbef9060d31 in _PyType_Lookup
at Objects/typeobject.c:3117
#10 0x00007fbef90461be in _PyObject_GenericGetAttrWithDict
at Objects/object.c:1231
#11 0x00007fbef9046707 in PyObject_GenericGetAttr
at Objects/object.c:1309
#12 0x00007fbef9043cdf in PyObject_GetAttr
at Objects/object.c:913
#13 0x00007fbef90458d9 in PyObject_GetAttrString
at Objects/object.c:818
#14 0x00007fbf02499636 in get_string_attr
at plpy_elog.c:569
#15 0x00007fbf02498ea5 in PLy_get_error_data
at plpy_elog.c:420
#16 0x00007fbf0249763b in PLy_elog_impl
at plpy_elog.c:77
Looks like there several places where code tries to get
attributes from error objects, and while code is ready for
attribute absence, it doesn't clear AttributeError exception
in that case.
Attached patch adds 3 calls to PyErr_Clear() in places where
code reacts on attribute absence. With this patch tests are
passed well.
There were similar findings before. Calls to PyErr_Clear were
close to, but not exactly at, same places before were removed
in
7e3bb08038 Fix access-to-already-freed-memory issue in plpython's error
handling.
Then one of PyErr_Clear were added in
1d2f9de38d Fix freshly-introduced PL/Python portability bug.
But looks like there's need for more.
PS. When python is compilled `--with-valgrind --without-pymalloc`
Valgrind doesn't complain, so there are no memory related
issues in plpython.
regards
------
Yura Sokolov
y.sokolov
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
index 7c627eacfbf..aa1cf17b366 100644
--- a/src/pl/plpython/plpy_elog.c
+++ b/src/pl/plpython/plpy_elog.c
@@ -359,7 +359,10 @@ PLy_get_sqlerrcode(PyObject *exc, int *sqlerrcode)
sqlstate = PyObject_GetAttrString(exc, "sqlstate");
if (sqlstate == NULL)
+ {
+ PyErr_Clear();
return;
+ }
buffer = PLyUnicode_AsString(sqlstate);
if (strlen(buffer) == 5 &&
@@ -395,6 +398,7 @@ PLy_get_spi_error_data(PyObject *exc, int *sqlerrcode, char **detail,
}
else
{
+ PyErr_Clear();
/*
* If there's no spidata, at least set the sqlerrcode. This can happen
* if someone explicitly raises a SPI exception from Python code.
@@ -571,6 +575,10 @@ get_string_attr(PyObject *obj, char *attrname, char **str)
{
*str = pstrdup(PLyUnicode_AsString(val));
}
+ else if (val == NULL)
+ {
+ PyErr_Clear();
+ }
Py_XDECREF(val);
}
