Re: Fix for segfault in plpython's exception handling

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Rafa de la Torre <rtorre(at)carto(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Fix for segfault in plpython's exception handling
Date: 2016-12-09 20:33:57
Message-ID: 29950.1481315637@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Rafa de la Torre <rtorre(at)carto(dot)com> writes:
>> exc = PyErr_NewException(exception_map[i].name, base, dict);
>> + Py_INCREF(exc);
>> PyModule_AddObject(mod, exception_map[i].classname, exc);

> Hm. Seems like if this is a problem, the code for the other three
> exceptions is being a bit careless: it does do Py_INCREFs on them,
> but not soon enough to ensure no problems. Also, I wonder why that
> code checks for a null result from PyErr_NewException but this doesn't.

> Good catch though. A naive person would have assumed that
> PyModule_AddObject would increment the object's refcount, but
> the Python docs say "This steals a reference to value", which
> I guess must mean that the caller is required to do it.

For me (testing with Python 2.6.6 on RHEL6), this test case didn't result
in a server crash, but in the wrong exception object name being reported.
Tracing through it showed that a python GC was happening during the loop
adding all the exceptions to the spiexceptions module, so that some of the
exception objects went away and then were immediately reallocated as other
exception objects. The explicit gc call in the test case wasn't
necessary, because the problem happened before that. Fun fun.

I've pushed a patch that deals with all these problems. Thanks for
the report!

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Rijkers 2016-12-09 21:00:04 Re: Logical Replication WIP
Previous Message Jeff Janes 2016-12-09 20:33:25 Re: new table partitioning breaks \d table to older versions