On Jan 10, 2008, at 8:58 PM, Chris McDonough wrote:

Hi folks,

Your note today pointing at code I wrote made me pay closer attention. :)

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 ...
  if dflag:

I actually think this should be:

  dflag = not getattr(ob, '_p_changed', None)
  ... stuff ...
  if dflag:

.. 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 heck.



Jim Fulton
Zope Corporation

Zope-Dev maillist  -  Zope-Dev@zope.org
**  No cross posts or HTML encoding!  **
(Related lists - http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )

Reply via email to