On Jan 10, 2008, at 8:58 PM, Chris McDonough wrote:
Your note today pointing at code I wrote made me pay closer
While messing with Zope2's webdav implementation, I ran across this
bit of memory-management code:
dflag = hasattr(ob, '_p_changed') and (ob._p_changed == None)
... stuff ...
I actually think this should be:
dflag = not getattr(ob, '_p_changed', None)
... stuff ...
.. because when _p_changed on a persistent object (as I read it in
the persistence interface file) is None, the object is a ghost. The
object will never be a ghost here because non-_p_ attributes are
looked up on it before we check for _p_changed, and even if it was a
ghost, deactivating a ghost (to turn it into a ghost) is just not
useful. I think the only time we don't want to deactivate it is if
it has been changed (when _p_changed is True). Or at least that
seems to be the intent.
Does this sound right?
The important thing to note above is that the check is made *before*
".. stuff ..". The idea is that we want to free any objects we loaded
while doing whatever we are doing. We check if an object is a ghost
before doing something and then, if it was a ghost before, we try to
make it a ghost when we're done. We try to leave the object as we
found it. With your change, we'd be ghostifying objects that were
presumably useful, because they were in the cache, and we'd be failing
to ghostify objects that we resurrected just for the operation.
I have a feeling the answer will be crickets, but I figure what the
Zope-Dev maillist - Zope-Dev@zope.org
** No cross posts or HTML encoding! **
(Related lists -