[Zodb-checkins] CVS: ZODB4/src/persistence - persistenceAPI.h:1.3
persistence.c:1.17
Jeremy Hylton
jeremy at zope.com
Tue May 20 16:01:40 EDT 2003
Update of /cvs-repository/ZODB4/src/persistence
In directory cvs.zope.org:/tmp/cvs-serv18494/persistence
Modified Files:
persistenceAPI.h persistence.c
Log Message:
Refactor persistence api to use _p_changed only to mark an object as changed.
Use _p_deactivate() to turn an object into a ghost, and use the
keyword argument force=1 if you want to turn a modified object into a
ghost. Several occurrences of the old interface have been updated.
This refactoring uncovered a number of subtle bugs in the persistence
C API. The two chief problems were that the load function in the C
API struct did not set the state and that the functions return 0 for
error and 1 for success. Regardless of whether these APIs are doing
the right thing, fix the code to use them correctly.
One downside of the new API is the C objects (BTrees) that override
_p_deactivate() have to deal with all the cruft for keyword
arguments. Since BTrees only add a single line of extra code to
_p_deactivate(), it seems useful to provide a hook in the persistence
framework for this purpose.
Also:
If an object is in the changed state, don't call register() on its
data manager a second time.
Ignore state changes that result from setstate() being called.
Don't load an object's state to call __setstate__().
In BTrees check module, if an object has an oid, print that along with
its id().
=== ZODB4/src/persistence/persistenceAPI.h 1.2 => 1.3 ===
--- ZODB4/src/persistence/persistenceAPI.h:1.2 Wed Dec 25 09:12:13 2002
+++ ZODB4/src/persistence/persistenceAPI.h Tue May 20 15:01:39 2003
@@ -38,25 +38,27 @@
#define PyPersist_IS_STICKY(O) \
((O)->po_state == STICKY || (O)->po_state == CHANGED)
-#define PyPersist_CHANGED(O) \
- PyPersist_C_API->reg_mgr((PyPersistObject *)(O))
+#define PyPersist_CHANGED(O) \
+ (PyPersist_C_API->reg_mgr((PyPersistObject *)(O)) ? \
+ ((PyPersistObject *)(O))->po_state = CHANGED, 1 : 0)
#define PyPersist_SetATime(O) \
PyPersist_C_API->set_atime((PyPersistObject *)(O))
/* Macros for compatibility with ZODB 3 C extensions. */
-#define PER_USE_OR_RETURN(O, R) \
-{ \
- if (((O)->po_state == GHOST) \
- && (!PyPersist_C_API->load((PyPersistObject *)(O)))) { \
- (O)->po_state = STICKY; \
- return (R); \
- } else if ((O)->po_state == UPTODATE) \
- (O)->po_state = STICKY; \
+#define PER_USE_OR_RETURN(O, R) \
+{ \
+ if ((O)->po_state == GHOST) { \
+ if (!PyPersist_C_API->load((PyPersistObject *)(O))) \
+ return (R); \
+ (O)->po_state = STICKY; \
+ } else if ((O)->po_state == UPTODATE) \
+ (O)->po_state = STICKY; \
}
-#define PER_CHANGED(O) PyPersist_C_API->reg_mgr((PyPersistObject *)(O))
+#define PER_CHANGED(O) \
+ PyPersist_C_API->reg_mgr((PyPersistObject *)(O)) ? -1 : 0
#define PER_ALLOW_DEACTIVATION(O) \
{ \
@@ -70,10 +72,17 @@
(O)->po_state = STICKY; \
}
-#define PER_USE(O) \
- ((((PyPersistObject *)(O))->po_state != GHOST) \
- || (PyPersist_C_API->load((PyPersistObject *)(O))) \
- ? ((((PyPersistObject *)(O))->po_state == UPTODATE) \
- ? (((PyPersistObject *)(O))->po_state = STICKY) : 1) : 0)
+/* Macro to load object and mark sticky as needed.
+
+ If the object is in the UPTODATE state, the mark it sticky.
+ If the object is in the GHOST state, load it and mark it sticky.
+ */
+
+#define PER_USE(O) \
+ (((PyPersistObject *)(O))->po_state != GHOST ? \
+ (((PyPersistObject *)(O))->po_state == UPTODATE ? \
+ ((PyPersistObject *)(O))->po_state = STICKY, 1 : 1) : \
+ (PyPersist_C_API->load((PyPersistObject *)(O)) ? \
+ ((PyPersistObject *)(O))->po_state = STICKY, 1 : 0))
#define PER_ACCESSED(O) PyPersist_C_API->set_atime((PyPersistObject *)O)
=== ZODB4/src/persistence/persistence.c 1.16 => 1.17 ===
--- ZODB4/src/persistence/persistence.c:1.16 Wed May 7 09:10:53 2003
+++ ZODB4/src/persistence/persistence.c Tue May 20 15:01:39 2003
@@ -43,7 +43,10 @@
PyObject *meth, *arg, *result;
if (!self->po_dm)
- return 0;
+ return 1;
+ /* If the object is in the CHANGED state, then it is already registered. */
+ if (self->po_state == CHANGED)
+ return 1;
if (!s_register)
s_register = PyString_InternFromString("register");
meth = PyObject_GetAttr((PyObject *)self->po_dm, s_register);
@@ -60,12 +63,11 @@
Py_DECREF(arg);
Py_DECREF(meth);
if (result) {
- if (self->po_state == UPTODATE || self->po_state == STICKY)
- self->po_state = CHANGED;
+ if (self->po_state == UPTODATE || self->po_state == STICKY)
+ self->po_state = CHANGED;
Py_DECREF(result);
return 1;
- }
- else
+ } else
return 0;
}
@@ -77,6 +79,7 @@
{
static PyObject *s_setstate = NULL;
PyObject *meth, *arg, *result;
+ enum PyPersist_State state;
if (self->po_dm == NULL)
return 0;
@@ -94,7 +97,13 @@
Py_INCREF(self);
PyTuple_SET_ITEM(arg, 0, (PyObject *)self);
+ /* set state to CHANGED while setstate() call is in progress
+ to prevent a recursive call to _PyPersist_Load().
+ */
+ state = self->po_state;
+ self->po_state = CHANGED;
result = PyObject_Call(meth, arg, NULL);
+ self->po_state = state;
Py_DECREF(arg);
Py_DECREF(meth);
@@ -240,16 +249,46 @@
}
static PyObject *
-persist_deactivate(PyPersistObject *self)
+persist_deactivate(PyPersistObject *self, PyObject *args, PyObject *keywords)
{
- if (self->po_state == UPTODATE && self->po_dm && self->po_oid) {
- PyObject **pdict = _PyObject_GetDictPtr((PyObject *)self);
- if (pdict && *pdict) {
- Py_DECREF(*pdict);
- *pdict = NULL;
+ int ghostify = 1;
+ PyObject *force = NULL;
+
+ if (args && PyTuple_GET_SIZE(args) > 0) {
+ PyErr_SetString(PyExc_TypeError,
+ "_p_deactivate takes not positional arguments");
+ return NULL;
+ }
+ if (keywords) {
+ int size = PyDict_Size(keywords);
+ force = PyDict_GetItemString(keywords, "force");
+ if (force)
+ size--;
+ if (size) {
+ PyErr_SetString(PyExc_TypeError,
+ "_p_deactivate only accepts keyword arg force");
+ return NULL;
}
- self->po_state = GHOST;
}
+
+ if (self->po_dm && self->po_oid) {
+ ghostify = self->po_state == UPTODATE;
+ if (!ghostify && force) {
+ if (PyObject_IsTrue(force))
+ ghostify = 1;
+ if (PyErr_Occurred())
+ return NULL;
+ }
+ if (ghostify) {
+ PyObject **pdict = _PyObject_GetDictPtr((PyObject *)self);
+ if (pdict && *pdict) {
+ Py_DECREF(*pdict);
+ *pdict = NULL;
+ }
+ self->po_state = GHOST;
+ }
+ }
+
Py_INCREF(Py_None);
return Py_None;
}
@@ -293,13 +332,16 @@
#define CHANGED_NONE 0
#define CHANGED_FALSE 1
#define CHANGED_TRUE 2
-#define CHANGED_DELETE 3
static int
persist_set_state(PyPersistObject *self, PyObject *v)
{
+ int newstate, bool;
- int newstate;
+ if (!v) {
+ PyErr_SetString(PyExc_TypeError, "can't delete _p_changed");
+ return -1;
+ }
/* If the object isn't registered with a data manager, setting its
state is meaningless.
@@ -307,14 +349,10 @@
if (!self->po_dm || !self->po_oid)
return 0;
- if (v == Py_None)
- newstate = CHANGED_NONE;
- else if (v == NULL)
- newstate = CHANGED_DELETE;
- else if (PyObject_IsTrue(v))
- newstate = CHANGED_TRUE;
- else
- newstate = CHANGED_FALSE;
+ bool = PyObject_IsTrue(v);
+ if (PyErr_Occurred())
+ return -1;
+ newstate = bool ? CHANGED_TRUE : CHANGED_FALSE;
/* XXX I think the cases below cover all the transitions of
interest. We should really extend the interface / documentation
@@ -344,13 +382,6 @@
*/
if (self->po_state == CHANGED || self->po_state == STICKY)
self->po_state = UPTODATE;
- } else if (newstate == CHANGED_DELETE) {
- /* Force the object to UPTODATE state to guarantee that
- call_p_deactivate() will turn it into a ghost.
- */
- self->po_state = UPTODATE;
- if (!call_p_deactivate(self, 0))
- return -1;
} else if (self->po_state == UPTODATE) {
/* The final case is for CHANGED_NONE, which is only
meaningful when the object is already in the up-to-date state.
@@ -407,7 +438,7 @@
*/
static int
-persist_checkattr(const char *s)
+persist_check_getattr(const char *s)
{
if (*s++ != '_')
return 1;
@@ -420,16 +451,21 @@
}
else if (*s == '_') {
s++;
- if (*s == 'd') {
+ switch (*s) {
+ case 'd':
s++;
if (!strncmp(s, "ict__", 5))
return 0; /* __dict__ */
if (!strncmp(s, "el__", 4))
return 0; /* __del__ */
return 1;
+ case 'c':
+ return strncmp(s, "class__", 7);
+ case 's':
+ return strncmp(s, "setstate__", 10);
+ default:
+ return 1;
}
- else if (!strncmp(s, "class__", 7))
- return 0; /* __class__ */
}
return 1;
}
@@ -452,7 +488,7 @@
underscore.
*/
- if (persist_checkattr(s_name)) {
+ if (persist_check_getattr(s_name)) {
if (self->po_state == GHOST) {
/* Prevent the object from being registered as changed.
@@ -501,6 +537,31 @@
1 : state loaded, attribute name is normal
*/
+/* Returns true if the object state must be loaded in setattr.
+
+ If any attribute other than _p_*, _v_*, or __dict__ is set,
+ the object must be unghostified.
+*/
+
+static int
+persist_check_setattr(const char *s)
+{
+ assert(s && *s);
+ if (*s++ != '_')
+ return 1;
+ switch (*s++) {
+ case 'p':
+ case 'v':
+ return *s != '_';
+ break;
+ case '_':
+ return strcmp(s, "dict__");
+ break;
+ default:
+ return 1;
+ }
+}
+
static int
persist_setattr_prep(PyPersistObject *self, PyObject *name, PyObject *value)
{
@@ -515,16 +576,7 @@
and excludes _p_ and _v_ attributes from the pickle.
*/
- /* If any attribute other than an _p_ or _v_ attribute or __dict__ is
- being assigned to, make sure that the object state is loaded.
-
- Implement with simple check on s_name[0] to avoid multiple strncmp()
- calls for all attribute names that don't start with an underscore.
- */
- if ((s_name[0] != '_')
- || ((strncmp(s_name, "_p_", 3) != 0)
- && (strncmp(s_name, "_v_", 3) != 0)
- && (strcmp(s_name, "__dict__") != 0))) {
+ if (persist_check_setattr(s_name)) {
if (self->po_state == GHOST) {
if (self->po_dm == NULL || self->po_oid == NULL) {
PyErr_SetString(PyExc_TypeError,
@@ -533,6 +585,7 @@
}
if (!_PyPersist_Load((PyPersistObject *)self))
return -1;
+ self->po_state = UPTODATE;
}
/* If the object is marked as UPTODATE then it must be
registered as modified. If it was just unghosted, it
@@ -541,6 +594,8 @@
If it's in the changed state, it should already be registered.
XXX What if it's in the sticky state?
+
+ XXX It looks like these two cases could be collapsed somehow.
*/
if (self->po_state == UPTODATE && self->po_dm &&
!_PyPersist_RegisterDataManager((PyPersistObject *)self))
@@ -721,7 +776,7 @@
{"__getstate__", (PyCFunction)persist_getstate, METH_NOARGS, },
{"__setstate__", persist_setstate, METH_O, },
{"_p_activate", (PyCFunction)persist_activate, METH_NOARGS, },
- {"_p_deactivate", (PyCFunction)persist_deactivate, METH_NOARGS, },
+ {"_p_deactivate", (PyCFunction)persist_deactivate, METH_KEYWORDS, },
{"_p_setattr", (PyCFunction)persist_p_setattr, METH_VARARGS, },
{"_p_delattr", (PyCFunction)persist_p_delattr, METH_O, },
{NULL}
More information about the Zodb-checkins
mailing list