[Zodb-checkins] CVS: Zope3/lib/python/Persistence - cPersistence.c:1.1.2.8

Jeremy Hylton jeremy@zope.com
Wed, 20 Feb 2002 18:39:45 -0500


Update of /cvs-repository/Zope3/lib/python/Persistence
In directory cvs.zope.org:/tmp/cvs-serv18065/lib/python/Persistence

Modified Files:
      Tag: Zope-3x-branch
	cPersistence.c 
Log Message:
Many changes -- enough for Persistence tests to pass.

Call PyObject_GC_Del() in persist_dealloc()!  The old version of the
code was decrefing all the attributes but not deleting the object.
This left an object with bogus pointers in the gc list.  No wonder if
dumped core.

Change name from persist_type to PyPersist_Type.
Re-indent PyPersist_Type definition.
Put function return type on separate line.

Share s_register, pointer to Python string "register," between two
different C functions.

Make several _PyPersist functions into PyPersist functions and add
them to the PyPersist_C_API_struct.  Expose in module a PyCObject
under name C_API.

Add PyPersist_RegisterTransaction(), part of C_API.
Change PyPersist_Register() to PyPersist_RegisterDataManager().

Change _p_atime from time_t to seconds % 86400.  Add comment that
explains that this is a temporary implementation accident.  Eventually
the basic Persistence machinery won't have cache-specific slots.
Eliminates needs for getset for _p_atime; a simple PyMemberDef will do.

Never return None from __getstate__().  It seemed to cause a lot of
inexplicable problems, probably caused by __setstate__() being
unprepared for it.  Always return an empty dict for an uninitialized
object.

Add missing braces around two statements indented under a PyDict_Next
while loop.




=== Zope3/lib/python/Persistence/cPersistence.c 1.1.2.7 => 1.1.2.8 ===
 "$Id$\n";
 
-staticforward PyTypeObject persist_type;
+staticforward PyTypeObject PyPersist_Type;
 
 /* A helper for setstate that is more efficient than PyMapping_Keys().
  */
 
-PyObject *_PyPersist_MappingKeys(PyObject *map)
+PyObject *
+_PyPersist_MappingKeys(PyObject *map)
 {
     static PyObject *s_keys = NULL;
     PyObject *meth, *args, *keys;
@@ -36,9 +37,11 @@
    manager.
 */
 
-PyObject *_PyPersist_Register(PyPersistObject *self) 
+static PyObject *s_register = NULL;
+
+PyObject *
+PyPersist_RegisterDataManager(PyPersistObject *self) 
 {
-    static PyObject *s_register = NULL;
     PyObject *meth, *arg, *result;
 
     if (self->po_dm == NULL) {
@@ -66,7 +69,8 @@
 /* A helper function that loads an object's state from its data manager.
 */
 
-PyObject *_PyPersist_Load(PyPersistObject *self) 
+PyObject *
+PyPersist_Load(PyPersistObject *self) 
 {
     static PyObject *s_setstate = NULL;
     PyObject *meth, *arg, *result;
@@ -93,6 +97,96 @@
     return result;
 }
 
+/* A helper function to register an object with the current
+   transaction.  Returns 1 if the object is successfully registered.
+   Returns 0 if the object doesn't need to be registered.  Returns -1
+   on error.
+*/
+
+int
+PyPersist_RegisterTransaction(PyPersistObject *self)
+{
+    static PyObject *get_transaction = NULL;
+    PyObject *mainmod = NULL, *builtins = NULL;
+    PyObject *args, *trans, *reg, *ret;
+
+    if (!((self->po_state == UPTODATE || self->po_state == STICKY)
+	  && self->po_dm))
+	return 0;
+
+    /* If get_transaction is not NULL, then s_register is not either. */
+    if (get_transaction == NULL) {
+	if (s_register == NULL) {
+	    s_register = PyString_InternFromString("register");
+	    if (s_register == NULL)
+		return -1;
+	}
+	mainmod = PyImport_ImportModule("__main__");
+	if (mainmod == NULL)
+	    return -1;
+	builtins = PyObject_GetAttrString(mainmod, "__builtins__");
+	if (builtins == NULL)
+	    goto get_transaction_error;
+	get_transaction = PyObject_GetAttrString(builtins, "get_transaction");
+	if (get_transaction == NULL)
+	    goto get_transaction_error;
+	Py_DECREF(builtins);
+	Py_DECREF(mainmod);
+    }
+    /* The C code below is equivalent to this Python code:
+       get_transaction().register(self)
+    */
+    args = PyTuple_New(0);
+    if (args == NULL)
+	return -1;
+    trans = PyObject_Call(get_transaction, args, NULL);
+    Py_DECREF(args);
+    args = NULL;
+    if (trans == NULL)
+	return -1;
+    reg = PyObject_GetAttr(trans, s_register);
+    if (reg == NULL)
+	goto register_error;
+    args = PyTuple_New(1);
+    if (args == NULL)
+	goto register_error;
+    PyTuple_SET_ITEM(args, 0, (PyObject *)self);
+    ret = PyObject_Call(reg, args, NULL);
+    PyTuple_SET_ITEM(args, 0, NULL);
+    if (ret == NULL)
+	goto register_error;
+    Py_DECREF(ret);
+    Py_DECREF(args);
+    Py_DECREF(reg);
+    Py_DECREF(trans);
+    self->po_state = CHANGED;
+    return 1;
+
+ register_error:
+    Py_XDECREF(args);
+    Py_XDECREF(trans);
+    Py_XDECREF(reg);
+    return -1;
+
+ get_transaction_error:
+    Py_XDECREF(mainmod);
+    Py_XDECREF(builtins);
+    return -1;
+}
+
+/* A helper function to set the atime from the current time.  The
+   po_atime slot stores seconds since the start of the day.  The need
+   for an atime slot and its particular semantics are specific to the
+   current cache implementation.
+ */
+
+void
+PyPersist_SetATime(PyPersistObject *self)
+{
+    time_t t = time(NULL);
+    self->po_atime = t % 86400;
+}
+
 static PyObject *
 persist_getstate(PyObject *self)
 {
@@ -100,19 +194,17 @@
     PyObject *state, *k, *v;
     int pos = 0;
 
-    /* If the instance doesn't have any attributes, the dict ptr could
-       still be NULL. */
-    if ((*pdict) == NULL) {
-	Py_INCREF(Py_None);
-	return Py_None;
-    }
-
     /* XXX UPDATE_STATE_IF_NECESSARY */
 
     state = PyDict_New();
     if (state == NULL)
 	return NULL;
 
+    /* If the instance doesn't have any attributes, the dict ptr could
+       still be NULL. */
+    if ((*pdict) == NULL)
+	return state;
+
     while (PyDict_Next(*pdict, &pos, &k, &v)) {
 	if (PyString_Check(k)) {
 	    char *attrname = PyString_AS_STRING(k);
@@ -155,7 +247,7 @@
 	PyObject *k, *v;
 	int pos = 0;
 
-	while (PyDict_Next(state, &pos, &k, &v)) 
+	while (PyDict_Next(state, &pos, &k, &v)) {
 	    if (PyString_Check(k)) {
 		char *attrname = PyString_AS_STRING(k);
 		if (strncmp(attrname, "_p_", 3) == 0)
@@ -163,6 +255,7 @@
 	    }
 	    if (PyDict_SetItem(dict, k, v) < 0)
 		return NULL;
+	}
     } else {
 	/* If it's not a dict, it may some other kind of mapping. 
 	   The old cPersistence supported this code, but I'm not sure
@@ -217,9 +310,13 @@
 {
     if (self->po_state == UPTODATE && self->po_dm) {
 	PyObject **pdict = _PyObject_GetDictPtr((PyObject *)self);
-	/* XXX should be safe to actually delete the dictionary */
-	if (*pdict)
+	if (*pdict) {
+/* Would prefer to free the dict, but need to extend other parts of
+   the implementation to check for the dict's existence.
+*/
+/*	    Py_DECREF(*pdict); */
 	    PyDict_Clear(*pdict);
+	}
 	self->po_state = GHOST;
     }
     Py_INCREF(Py_None);
@@ -236,6 +333,10 @@
 static PyObject *
 persist_get_state(PyPersistObject *self)
 {
+    if (self->po_state == GHOST) {
+	Py_INCREF(Py_None);
+	return Py_None;
+    }
     return PyInt_FromLong(self->po_state);
 }
 
@@ -272,7 +373,8 @@
     if (self->po_state == GHOST) {
 	if (newstate == CHANGED_TRUE || newstate == CHANGED_FALSE) {
 	    /* Turn a ghost into a real object. */
-	    if (_PyPersist_Load(self) == NULL)
+	    self->po_state = CHANGED;
+	    if (PyPersist_Load(self) == NULL)
 		return -1;
 	    if (newstate == CHANGED_TRUE)
 		self->po_state = CHANGED;
@@ -283,7 +385,7 @@
 	/* Mark an up-to-date object as changed. */
 	if (self->po_state == UPTODATE) {
 	    self->po_state = CHANGED;
-	    if (_PyPersist_Register(self) == NULL)
+	    if (PyPersist_RegisterDataManager(self) == NULL)
 		return -1;
 	}
     } else if (newstate == CHANGED_FALSE) {
@@ -309,32 +411,20 @@
     return 0;
 }
 
-
-static PyObject *
-persist_get_time(PyPersistObject *self)
-{
-    struct tm *tm = gmtime(&self->po_atime);
-    return PyInt_FromLong(((tm->tm_hour * 60 + tm->tm_min) * 60) + tm->tm_sec);
-}
-
-
 static PyObject *
 persist_get_dict(PyPersistObject *self)
 {
-    PyObject *d;
-    d = self->po_dict;
-    if (d == NULL) {
-	self->po_dict = d = PyDict_New();
-	if (d == NULL)
+    if (self->po_dict == NULL) {
+	self->po_dict = PyDict_New();
+	if (self->po_dict == NULL)
 	    return NULL;
     }
-    Py_INCREF(d);
-    return d;
+    Py_INCREF(self->po_dict);
+    return self->po_dict;
 }
 
 static PyGetSetDef persist_getsets[] = {
     {"_p_changed", (getter)persist_get_state, (setter)persist_set_state},
-    {"_p_time", (getter)persist_get_time},
     {"__dict__", (getter)persist_get_dict},
     {NULL}
 };
@@ -345,8 +435,9 @@
     {"_p_jar", T_OBJECT, offsetof(PyPersistObject, po_dm)},
     {"_p_oid", T_OBJECT, offsetof(PyPersistObject, po_oid)},
     {"_p_serial", T_OBJECT, offsetof(PyPersistObject, po_serial)},
-    /* XXX should state should be exposed? */
-    {"_p_state", T_INT, offsetof(PyPersistObject, po_state)},
+    {"_p_atime", T_INT, offsetof(PyPersistObject, po_atime)},
+    /* XXX should this be exposed? */
+    {"_p_state", T_INT, offsetof(PyPersistObject, po_state), RO},
     {NULL}
 };
 
@@ -395,14 +486,15 @@
 	|| ((strncmp(s_name, "_p_", 3) != 0)
 	    && (strcmp(s_name, "__dict__") != 0))) {
 	if (self->po_state == GHOST) {
-	    if (_PyPersist_Load(self) == NULL) {
+	    self->po_state = CHANGED;
+	    if (PyPersist_Load(self) == NULL) {
 		persist_deactivate(self);
 		self->po_state = GHOST;
 		return NULL;
 	    } else
 		self->po_state = UPTODATE;
 	}
-	self->po_atime = time(NULL);
+	PyPersist_SetATime(self);
     }
 
     attr = PyObject_GenericGetAttr((PyObject *)self, name);
@@ -451,15 +543,15 @@
 				"attempt to modify unrevivable ghost");
 		return -1;
 	    }
-	    if (_PyPersist_Load(self) == NULL)
+	    if (PyPersist_Load(self) == NULL)
 		return -1;
 	} else if (self->po_state == UPTODATE && self->po_dm)
-	    if (_PyPersist_Register(self) == NULL)
+	    if (PyPersist_RegisterDataManager(self) == NULL)
 		return -1;
 
 	if (self->po_dm && self->po_oid) {
 	    self->po_state = CHANGED;
-	    self->po_atime = time(NULL);
+	    PyPersist_SetATime(self);
 	}
     }
 
@@ -479,6 +571,7 @@
     Py_XDECREF(self->po_oid);
     Py_XDECREF(self->po_serial);
     Py_XDECREF(self->po_dict);
+    PyObject_GC_Del(self);
 }
 
 static int
@@ -523,60 +616,67 @@
 */
 #define DEFERRED_ADDRESS(ADDR) 0
 
-static PyTypeObject persist_type = {
-	PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
-	0,					/* ob_size */
-	"Persistent",				/* tp_name */
-	sizeof(PyPersistObject),		/* tp_basicsize */
-	0,					/* tp_itemsize */
-	(destructor)persist_dealloc,		/* tp_dealloc */
-	0,					/* tp_print */
-	0,					/* tp_getattr */
-	0,					/* tp_setattr */
-	0,					/* tp_compare */
-	0,					/* tp_repr */
-	0,					/* tp_as_number */
-	0,					/* tp_as_sequence */
-	0,					/* tp_as_mapping */
-	0,					/* tp_hash */
-	0,					/* tp_call */
-	0,					/* tp_str */
-	(getattrofunc)persist_getattro,		/* tp_getattro */
-	(setattrofunc)persist_setattro,		/* tp_setattro */
-	0,					/* tp_as_buffer */
-	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
-		Py_TPFLAGS_BASETYPE, 		/* tp_flags */
-	0,					/* tp_doc */
-	(traverseproc)persist_traverse,		/* tp_traverse */
-	(inquiry)persist_clear,			/* tp_clear */
-	0,					/* tp_richcompare */
-	0,					/* tp_weaklistoffset */
-	0,					/* tp_iter */
-	0,					/* tp_iternext */
-	persist_methods,			/* tp_methods */
-	persist_members,			/* tp_members */
-	persist_getsets,			/* tp_getset */
-	0,					/* tp_base */
-	0,					/* tp_dict */
-	0,					/* tp_descr_get */
-	0,					/* tp_descr_set */
-	offsetof(PyPersistObject, po_dict),	/* tp_dictoffset */
-	0,					/* tp_init */
-	0,					/* tp_alloc */
-	PyType_GenericNew,			/* tp_new */
+static PyTypeObject PyPersist_Type = {
+    PyObject_HEAD_INIT(DEFERRED_ADDRESS(&PyType_Type))
+    0,					/* ob_size */
+    "Persistent",			/* tp_name */
+    sizeof(PyPersistObject),		/* tp_basicsize */
+    0,					/* tp_itemsize */
+    (destructor)persist_dealloc,	/* tp_dealloc */
+    0,					/* tp_print */
+    0,					/* tp_getattr */
+    0,					/* tp_setattr */
+    0,					/* tp_compare */
+    0,					/* tp_repr */
+    0,					/* tp_as_number */
+    0,					/* tp_as_sequence */
+    0,					/* tp_as_mapping */
+    0,					/* tp_hash */
+    0,					/* tp_call */
+    0,					/* tp_str */
+    (getattrofunc)persist_getattro,	/* tp_getattro */
+    (setattrofunc)persist_setattro,	/* tp_setattro */
+    0,					/* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
+    Py_TPFLAGS_BASETYPE, 		/* tp_flags */
+    0,					/* tp_doc */
+    (traverseproc)persist_traverse,	/* tp_traverse */
+    (inquiry)persist_clear,		/* tp_clear */
+    0,					/* tp_richcompare */
+    0,					/* tp_weaklistoffset */
+    0,					/* tp_iter */
+    0,					/* tp_iternext */
+    persist_methods,			/* tp_methods */
+    persist_members,			/* tp_members */
+    persist_getsets,			/* tp_getset */
+    0,					/* tp_base */
+    0,					/* tp_dict */
+    0,					/* tp_descr_get */
+    0,					/* tp_descr_set */
+    offsetof(PyPersistObject, po_dict),	/* tp_dictoffset */
+    0,					/* tp_init */
+    0,					/* tp_alloc */
+    PyType_GenericNew,			/* tp_new */
 };
 
 static PyMethodDef PyPersist_methods[] = {
     {NULL, NULL}
 };
 
+static PyPersist_C_API_struct c_api = {
+    &PyPersist_Type,
+    PyPersist_Load,
+    PyPersist_RegisterTransaction,
+    PyPersist_SetATime
+};
+
 void 
 initcPersistence(void)
 {
-    PyObject *m, *d;
+    PyObject *m, *d, *v;
 
-    persist_type.ob_type = &PyType_Type;
-    if (PyType_Ready(&persist_type) < 0)
+    PyPersist_Type.ob_type = &PyType_Type;
+    if (PyType_Ready(&PyPersist_Type) < 0)
 	return;
 
     m = Py_InitModule3("cPersistence", PyPersist_methods, 
@@ -587,7 +687,14 @@
     if (d == NULL)
 	return;
 
-    Py_INCREF(&persist_type);
-    if (PyDict_SetItemString(d, "Persistent", (PyObject *)&persist_type) < 0)
+    Py_INCREF(&PyPersist_Type);
+    if (PyDict_SetItemString(d, "Persistent", (PyObject *)&PyPersist_Type) < 0)
+	return;
+
+    v = PyCObject_FromVoidPtr(&c_api, NULL);
+    if (v == NULL)
+	return;
+    if (PyDict_SetItemString(d, "C_API", v) < 0)
 	return;
+    Py_DECREF(v);
 }