Commit e8b9c8e9 authored by Jason Madden's avatar Jason Madden

Avoid raising a SystemError when clearing slots if setstate() failed.

PR #52 introduced a code path to `ghostify` that calls PyErr_Clear()
with the intent to avoid propagating AttributeErrors for slots.

However, if there is an error (like a POSKeyError) raised by
jar.setstate(), then `unghostify` will call ghostify with an error
pending. If the object had slots that weren't set and the
AttributeError was cleared, so was the pending error from setstate. So
when `ghostify` returned NULL that got propagated up to the
interpreter which finds no exception and so raises `SystemError: error
return without exception set`.

This commit makes `unghostify` save and restore the exception state
around the call to PyErr_Clear.
parent aba23595
......@@ -4,7 +4,9 @@
4.2.4 (unreleased)
------------------
- Nothing changed yet.
- Avoid raising a ``SystemError: error return without exception set``
when loading an object with slots whose jar generates an exception
(such as a ZODB ``POSKeyError``) in ``setstate``.
4.2.3 (2017-03-08)
......
......@@ -152,6 +152,7 @@ static void
ghostify(cPersistentObject *self)
{
PyObject **dictptr, *slotnames;
PyObject *errtype, *errvalue, *errtb;
/* are we already a ghost? */
if (self->state == cPersistent_GHOST_STATE)
......@@ -195,6 +196,12 @@ ghostify(cPersistentObject *self)
* override __new__ ) */
if (Py_TYPE(self)->tp_new == Pertype.tp_new)
{
/* later we might clear an AttributeError but
* if we have a pending exception that still needs to be
* raised so that we don't generate a SystemError.
*/
PyErr_Fetch(&errtype, &errvalue, &errtb);
slotnames = pickle_slotnames(Py_TYPE(self));
if (slotnames && slotnames != Py_None)
{
......@@ -235,6 +242,7 @@ ghostify(cPersistentObject *self)
}
}
Py_XDECREF(slotnames);
PyErr_Restore(errtype, errvalue, errtb);
}
/* We remove the reference to the just ghosted object that the ring
......
......@@ -16,6 +16,9 @@ import unittest
import platform
import sys
from persistent._compat import copy_reg
py_impl = getattr(platform, 'python_implementation', lambda: None)
_is_pypy3 = py_impl() == 'PyPy' and sys.version_info[0] > 2
_is_jython = py_impl() == 'Jython'
......@@ -78,22 +81,22 @@ class _Persistent_Base(object):
self.called = 0
def register(self,ob):
self.called += 1
raise NotImplementedError
raise NotImplementedError()
def setstate(self,ob):
raise NotImplementedError
raise NotImplementedError()
jar = _BrokenJar()
jar._cache = self._makeCache(jar)
return jar
def _makeOneWithJar(self, klass=None):
def _makeOneWithJar(self, klass=None, broken_jar=False):
from persistent.timestamp import _makeOctets
OID = _makeOctets('\x01' * 8)
if klass is not None:
inst = klass()
else:
inst = self._makeOne()
jar = self._makeJar()
jar = self._makeJar() if not broken_jar else self._makeBrokenJar()
jar._cache.new_ghost(OID, inst) # assigns _p_jar, _p_oid
return inst, jar, OID
......@@ -1401,6 +1404,26 @@ class _Persistent_Base(object):
self.assertEqual(list(jar._loaded), [])
self.assertEqual(list(jar._registered), [])
def test_p_invalidate_with_slots_broken_jar(self):
# If jar.setstate() raises a POSKeyError (or any error)
# clearing an object with unset slots doesn't result in a
# SystemError, the original error is propagated
class Derived(self._getTargetClass()):
__slots__ = ('slot1',)
# Pre-cache in __slotnames__; cpersistent goes directly for this
# and avoids a call to copy_reg. (If it calls the python code in
# copy_reg, the pending exception will be immediately propagated by
# copy_reg, not by us.)
copy_reg._slotnames(Derived)
inst, jar, OID = self._makeOneWithJar(Derived, broken_jar=True)
inst._p_invalidate()
self.assertEqual(inst._p_status, 'ghost')
self.assertRaises(NotImplementedError, inst._p_activate)
def test__p_invalidate_from_sticky(self):
inst, jar, OID = self._makeOneWithJar()
inst._p_activate() # XXX
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment