On 06/04/11 22:16, Jan Urbański wrote:
> On 06/04/11 21:38, Peter Eisentraut wrote:
>> On mån, 2011-03-21 at 00:40 +0100, Jan Urbański wrote:
>>> I finally got around to updating the PL/Python tracebacks patch. The
>>> other day I was writing some very simple PL/Python code and the lack of
>>> tracebacks is extremely annoying.
>>
>> I tweaked this a bit to make the patch less invasive, and then committed
>> it. :)
>
> Ouch, just today I found a flaw in this, namely that it assumes the
> lineno from the traceback always refers to the PL/Python function. If
> you create a PL/Python function that imports some code, runs it, and
> that code raises an exception, PLy_traceback will get utterly confused.
>
> Working on a fix...
Here's the fix.
The actual bug was funny. The traceback code was fetching the file line
from the traceback and trying to get that line from the original source
to print it. But sometimes that line was refering to a different source
file, like when the exception originated from an imported module.
In my testing I accidentally had the error (in a separate module) on
line 2, so the traceback code tried to fetch line 2 of the function,
which was completely whitespace. This can never happen in theory,
because you can't have a frame starting at an all-whitespace line. The
code to get that line was misbehaving and trying to do a malloc(-2),
which in turn was causing an "ERROR invalid memory allocation".
All that is fixed with the attached patch.
Cheers,
Jan
PS: and thanks for committing that in the first place! :)
J
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 9352580..b2333b8 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** get_source_line(const char *src, int lin
*** 4507,4512 ****
--- 4507,4520 ----
if (next == NULL)
return pstrdup(s);
+ /*
+ * Sanity check, next < s if the line was all-whitespace, which should
+ * never happen if Python reported an frame created on that line, but
+ * check anyway.
+ */
+ if (next < s)
+ return NULL;
+
return pnstrdup(s, next - s);
}
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4603,4608 ****
--- 4611,4617 ----
PyObject *volatile code = NULL;
PyObject *volatile name = NULL;
PyObject *volatile lineno = NULL;
+ PyObject *volatile filename = NULL;
PG_TRY();
{
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4621,4626 ****
--- 4630,4639 ----
name = PyObject_GetAttrString(code, "co_name");
if (name == NULL)
elog(ERROR, "could not get function name from Python code object");
+
+ filename = PyObject_GetAttrString(code, "co_filename");
+ if (filename == NULL)
+ elog(ERROR, "could not get file name from Python code object");
}
PG_CATCH();
{
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4628,4633 ****
--- 4641,4647 ----
Py_XDECREF(code);
Py_XDECREF(name);
Py_XDECREF(lineno);
+ Py_XDECREF(filename);
PG_RE_THROW();
}
PG_END_TRY();
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4638,4643 ****
--- 4652,4658 ----
char *proname;
char *fname;
char *line;
+ char *plain_filename;
long plain_lineno;
/*
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4651,4656 ****
--- 4666,4672 ----
fname = PyString_AsString(name);
proname = PLy_procedure_name(PLy_curr_procedure);
+ plain_filename = PyString_AsString(filename);
plain_lineno = PyInt_AsLong(lineno);
if (proname == NULL)
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4662,4668 ****
&tbstr, "\n PL/Python function \"%s\", line %ld, in %s",
proname, plain_lineno - 1, fname);
! if (PLy_curr_procedure)
{
/*
* If we know the current procedure, append the exact
--- 4678,4686 ----
&tbstr, "\n PL/Python function \"%s\", line %ld, in %s",
proname, plain_lineno - 1, fname);
! /* the code object was compiled with "<string>" as the filename */
! if (PLy_curr_procedure && plain_filename != NULL &&
! strcmp(plain_filename, "<string>") == 0)
{
/*
* If we know the current procedure, append the exact
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4670,4676 ****
* 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.
*/
line = get_source_line(PLy_curr_procedure->src, plain_lineno);
if (line)
--- 4688,4696 ----
* 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. However,
! * do not go as far as traceback.py in reading the source
! * of imported modules.
*/
line = get_source_line(PLy_curr_procedure->src, plain_lineno);
if (line)
*************** PLy_traceback(char **xmsg, char **tbmsg,
*** 4685,4690 ****
--- 4705,4711 ----
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;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
