On Sun, Jan 25, 2015 at 2:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This is scary as hell. I intend to go around and manually audit
> every single PG_TRY in the current source code, but that is obviously
> not a long-term solution. Anybody have an idea about how we might
> get trustworthy mechanical detection of this type of situation?
One idea I've been thinking about for a while is to create some new,
safer notation. Suppose we did this:
PG_TRY_WITH_CLEANUP(cleanup_function, cleanup_argument);
{ /* code requiring cleanup */
}
PG_END_TRY_WITH_CLEANUP();
Instead of doing anything with sigsetjmp(), this would just push a
frame onto a cleanup stack. We would call of those callbacks from
innermost to outermost before doing siglongjmp(). With this design,
we don't need any volatile-ization.
This doesn't work for PG_CATCH() blocks that do not PG_RE_THROW(), but
there are not a ton of those. In a quick search, I found initTrie,
do_autovacuum, xml_is_document, and a number of instances in various
procedural languages. Most instances in the core code could be
converted to the style above. Aside from any reduction in the need
for volatile, this might actually perform slightly better, because
sigsetjmp() is a system call on some platforms. There are probably
few cases where that actually matters, but the one in pq_getmessage(),
for example, might not be entirely discountable.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company