[Zodb-checkins] SVN: ZODB/trunk/src/ Bug Fixed:
Jim Fulton
jim at zope.com
Wed Dec 16 12:51:36 EST 2009
Log message for revision 106652:
Bug Fixed:
Object state management wasn't dome correctly when classes
implemented custom _p_deavtivate methods.
(https://bugs.launchpad.net/zodb/+bug/185066)
Changed:
U ZODB/trunk/src/CHANGES.txt
U ZODB/trunk/src/ZODB/Connection.py
U ZODB/trunk/src/ZODB/serialize.py
U ZODB/trunk/src/ZODB/tests/testConnection.py
U ZODB/trunk/src/persistent/cPickleCache.c
U ZODB/trunk/src/persistent/tests/test_PickleCache.py
-=-
Modified: ZODB/trunk/src/CHANGES.txt
===================================================================
--- ZODB/trunk/src/CHANGES.txt 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/CHANGES.txt 2009-12-16 17:51:35 UTC (rev 106652)
@@ -2,6 +2,16 @@
Change History
================
+3.10.0a1 (2009-12-??)
+=====================
+
+Bugs Fixed
+----------
+
+- Object state management wasn't dome correctly when classes
+ implemented custom _p_deavtivate methods.
+ (https://bugs.launchpad.net/zodb/+bug/185066)
+
3.9.4 (2009-12-14)
==================
Modified: ZODB/trunk/src/ZODB/Connection.py
===================================================================
--- ZODB/trunk/src/ZODB/Connection.py 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/ZODB/Connection.py 2009-12-16 17:51:35 UTC (rev 106652)
@@ -241,24 +241,16 @@
if obj is not None:
return obj
- # This appears to be an MVCC violation because we are loading
- # the must recent data when perhaps we shouldnt. The key is
- # that we are only creating a ghost!
- # A disadvantage to this optimization is that _p_serial cannot be
- # trusted until the object has been loaded, which affects both MVCC
- # and historical connections.
p, serial = self._storage.load(oid, '')
obj = self._reader.getGhost(p)
# Avoid infiniate loop if obj tries to load its state before
# it is added to the cache and it's state refers to it.
+ # (This will typically be the case for non-ghostifyable objects,
+ # like persistent caches.)
self._pre_cache[oid] = obj
- obj._p_oid = oid
- obj._p_jar = self
- obj._p_changed = None
- obj._p_serial = serial
+ self._cache.new_ghost(oid, obj)
self._pre_cache.pop(oid)
- self._cache[oid] = obj
return obj
def cacheMinimize(self):
Modified: ZODB/trunk/src/ZODB/serialize.py
===================================================================
--- ZODB/trunk/src/ZODB/serialize.py 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/ZODB/serialize.py 2009-12-16 17:51:35 UTC (rev 106652)
@@ -511,14 +511,7 @@
return self._conn.get(oid)
# TODO: should be done by connection
- obj._p_oid = oid
- obj._p_jar = self._conn
- # When an object is created, it is put in the UPTODATE
- # state. We must explicitly deactivate it to turn it into
- # a ghost.
- obj._p_changed = None
-
- self._cache[oid] = obj
+ self._cache.new_ghost(oid, obj)
return obj
def load_multi_persistent(self, database_name, oid, klass):
@@ -552,16 +545,6 @@
loaders['n'] = load_multi_oid
- def _new_object(self, klass, args):
- if not args and not myhasattr(klass, "__getnewargs__"):
- obj = klass.__new__(klass)
- else:
- obj = klass(*args)
- if not isinstance(klass, type):
- obj.__dict__.clear()
-
- return obj
-
def getClassName(self, pickle):
unpickler = self._get_unpickler(pickle)
klass = unpickler.load()
Modified: ZODB/trunk/src/ZODB/tests/testConnection.py
===================================================================
--- ZODB/trunk/src/ZODB/tests/testConnection.py 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/ZODB/tests/testConnection.py 2009-12-16 17:51:35 UTC (rev 106652)
@@ -556,6 +556,29 @@
<root: rather_long_name rather_long_name2 rather_long_name4 ...>
"""
+class proper_ghost_initialization_with_empty__p_deactivate_class(Persistent):
+ def _p_deactivate(self):
+ pass
+
+def proper_ghost_initialization_with_empty__p_deactivate():
+ """
+See https://bugs.launchpad.net/zodb/+bug/185066
+
+ >>> db = ZODB.tests.util.DB()
+ >>> conn = db.open()
+ >>> C = proper_ghost_initialization_with_empty__p_deactivate_class
+ >>> conn.root.x = x = C()
+ >>> conn.root.x.y = 1
+ >>> transaction.commit()
+
+ >>> conn2 = db.open()
+ >>> conn2.root.x._p_changed
+
+ >>> conn2.root.x.y
+ 1
+
+ """
+
class _PlayPersistent(Persistent):
def setValueWithSize(self, size=0): self.value = size*' '
__init__ = setValueWithSize
Modified: ZODB/trunk/src/persistent/cPickleCache.c
===================================================================
--- ZODB/trunk/src/persistent/cPickleCache.c 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/persistent/cPickleCache.c 2009-12-16 17:51:35 UTC (rev 106652)
@@ -691,7 +691,104 @@
Py_RETURN_NONE;
}
+static PyObject*
+cc_new_ghost(ccobject *self, PyObject *args)
+{
+ PyObject *tmp, *key, *v;
+ if (!PyArg_ParseTuple(args, "OO:new_ghost", &key, &v))
+ return NULL;
+
+ /* Sanity check the value given to make sure it is allowed in the cache */
+ if (PyType_Check(v))
+ {
+ /* Its a persistent class, such as a ZClass. Thats ok. */
+ }
+ else if (v->ob_type->tp_basicsize < sizeof(cPersistentObject))
+ {
+ /* If it's not an instance of a persistent class, (ie Python
+ classes that derive from persistent.Persistent, BTrees,
+ etc), report an error.
+
+ TODO: checking sizeof() seems a poor test.
+ */
+ PyErr_SetString(PyExc_TypeError,
+ "Cache values must be persistent objects.");
+ return NULL;
+ }
+
+ /* Can't access v->oid directly because the object might be a
+ * persistent class.
+ */
+ tmp = PyObject_GetAttr(v, py__p_oid);
+ if (tmp == NULL)
+ return NULL;
+ Py_DECREF(tmp);
+ if (tmp != Py_None)
+ {
+ PyErr_SetString(PyExc_AssertionError,
+ "New ghost object must not have an oid");
+ return NULL;
+ }
+
+ /* useful sanity check, but not strictly an invariant of this class */
+ tmp = PyObject_GetAttr(v, py__p_jar);
+ if (tmp == NULL)
+ return NULL;
+ Py_DECREF(tmp);
+ if (tmp != Py_None)
+ {
+ PyErr_SetString(PyExc_AssertionError,
+ "New ghost object must not have a jar");
+ return NULL;
+ }
+
+ tmp = PyDict_GetItem(self->data, key);
+ if (tmp)
+ {
+ Py_DECREF(tmp);
+ PyErr_SetString(PyExc_AssertionError,
+ "The given oid is already in the cache");
+ return NULL;
+ }
+
+ if (PyType_Check(v))
+ {
+ if (PyObject_SetAttr(v, py__p_jar, self->jar) < 0)
+ return NULL;
+ if (PyObject_SetAttr(v, py__p_oid, key) < 0)
+ return NULL;
+ if (PyDict_SetItem(self->data, key, v) < 0)
+ return NULL;
+ self->klass_count++;
+ }
+ else
+ {
+ cPersistentObject *p = (cPersistentObject *)v;
+
+ if(p->cache != NULL)
+ {
+ PyErr_SetString(PyExc_AssertionError, "Already in a cache");
+ return NULL;
+ }
+
+ if (PyDict_SetItem(self->data, key, v) < 0)
+ return NULL;
+ /* the dict should have a borrowed reference */
+ Py_DECREF(v);
+
+ Py_INCREF(self);
+ p->cache = (PerCache *)self;
+ Py_INCREF(self->jar);
+ p->jar = self->jar;
+ Py_INCREF(key);
+ p->oid = key;
+ p->state = cPersistent_GHOST_STATE;
+ }
+
+ Py_RETURN_NONE;
+}
+
static struct PyMethodDef cc_methods[] = {
{"items", (PyCFunction)cc_items, METH_NOARGS,
"Return list of oid, object pairs for all items in cache."},
@@ -724,6 +821,8 @@
"update_object_size_estimation(oid, new_size) -- "
"update the caches size estimation for *oid* "
"(if this is known to the cache)."},
+ {"new_ghost", (PyCFunction)cc_new_ghost, METH_VARARGS,
+ "new_ghost() -- Initialize a ghost and add it to the cache."},
{NULL, NULL} /* sentinel */
};
Modified: ZODB/trunk/src/persistent/tests/test_PickleCache.py
===================================================================
--- ZODB/trunk/src/persistent/tests/test_PickleCache.py 2009-12-16 17:49:23 UTC (rev 106651)
+++ ZODB/trunk/src/persistent/tests/test_PickleCache.py 2009-12-16 17:51:35 UTC (rev 106652)
@@ -40,6 +40,80 @@
"""
+def new_ghost():
+ """
+Creating ghosts (from scratch, as opposed to ghostifying a non-ghost)
+in the curremt implementation is rather tricky. IPeristent doesn't
+really provide the right interface given that:
+
+- _p_deactivate and _p_invalidate are overridable and could assume
+ that the object's state is properly initialized.
+
+- Assigning _p_changed to None or deleting it just calls _p_deactivate
+ or _p_invalidate.
+
+The current cache implementation is intimately tied up with the
+persistence implementation and has internal access to the persistence
+state. The cache implementation can update the persistence state for
+newly created and ininitialized objects directly.
+
+The future persistence and cache implementations will be far more
+decoupled. The persistence implementation will only manage object
+state and generate object-usage events. The cache implemnentation(s)
+will be rersponsible for managing persistence-related (meta-)state,
+such as _p_state, _p_changed, _p_oid, etc. So in that future
+implemention, the cache will be more central to managing object
+persistence information.
+
+Caches have a new_ghost method that:
+
+- adds an object to the cache, and
+- initializes its persistence data.
+
+ >>> import persistent
+
+ >>> class C(persistent.Persistent):
+ ... pass
+
+ >>> jar = object()
+ >>> cache = persistent.PickleCache(jar, 10, 100)
+ >>> ob = C.__new__(C)
+ >>> cache.new_ghost('1', ob)
+
+ >>> ob._p_changed
+ >>> ob._p_jar is jar
+ True
+ >>> ob._p_oid
+ '1'
+
+ >>> cache.cache_non_ghost_count, cache.total_estimated_size
+ (0, 0)
+
+
+Peristent meta classes work too:
+
+ >>> import ZODB.persistentclass
+ >>> class PC:
+ ... __metaclass__ = ZODB.persistentclass.PersistentMetaClass
+
+ >>> PC._p_oid
+ >>> PC._p_jar
+ >>> PC._p_serial
+ >>> PC._p_changed
+ False
+
+ >>> cache.new_ghost('2', PC)
+ >>> PC._p_oid
+ '2'
+ >>> PC._p_jar is jar
+ True
+ >>> PC._p_serial
+ >>> PC._p_changed
+ False
+
+ """
+
+
from zope.testing.doctest import DocTestSuite
import unittest
More information about the Zodb-checkins
mailing list