Comment 2 for bug 586526

On Thu, May 27, 2010 at 4:00 PM, Jacob Holm <email address hidden> wrote:
> Public bug reported:
>
> >From persistent/interfaces.py:
>
>    There are two ways to invalidate an object:  call the
>    _p_invalidate() method (preferred) or delete its _p_changed
>    attribute.

And, of course, ZODB itself does the later. :)

> It turns out that these two are not equivalent when the object has a
> custom _p_invalidate method.

Gaaaa. Why do you want to override _p_invalidate?

>   Specifically, the custom _p_invalidate
> method is never called when you do delete the _p_changed attribute.
> (Similarly, a custom _p_activate is never called when unghostifying an
> object, but I don't have a use case for that that isn't already covered
> by having a custom __setstate__)
>
> Looking at cPersistence.c it is immediately obvious why.  The way things
> are implemented,  Persistent._p_invalidate() uses the equivalent of "del
> self._p_changed" which of course doesn't call _p_invalidate again.
> Instead it sets the state to UPTODATE and calls _p_deactivate.   (Why it
> does that is another matter, I think lying about the state when
> potentially calling user code is *BAD*)
>
> I think it would be better if that was reversed so Per_set_changed would
> call the objects (possibly custom) _p_invalidate, which would then set
> the state and call _p_deactivate itself.    This would not only fix the
> immediate problem, but also make the control flow more obvious.
> (Modifying _p_changed dispatches to either _p_deactivate or
> _p_invalidate.  _p_invalidate sets state to UPTODATE, calls
> _p_deactivate, and then forces the object to become a ghost)
>
> Would a patch along those lines be acceptable for 3.10?

This whole implementation is such a mess wrt custom _p_invalidate and
_p_deactivate that I really don't want to touch it until there's a
significant do over.

The way this should have been done was to have something like:
_p_release_state that is called by _p_invalidate and _p_deactivate.

Better yet, I might even replace _p_invalidate with a flag indicating
whether the object was ghostifiable.

In any case, I have very little interest in fooling with the C
persistence implementation at this time.

>   Or should I
> rather concentrate on a patch that eliminates all *uses* of "del
> p._changed" from ZODB  (a one-line change in ZODB/Connection.py), and
> hope that none of my other dependencies use it?

That might be better, although I don't think this is the only place
where del is used.

Jim

--
Jim Fulton