_p_changed is stuck if register() fails
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
ZODB |
Confirmed
|
Wishlist
|
Unassigned |
Bug Description
If an error occurs during the call to a data manager's register() method when setting _p_changed, the persistent object's po_state is left in the "CHANGED" state, even though the object has not actually been changed. The state should remain "UPTODATE", otherwise a subsequent change to the object will not result in a repeat call to the data manager's register() method.
When an object's state is changed directly, cPersistence does not update po_state until register() has successfully returned. The code for handling direct assignment to _p_changed should do likewise. Moving the line:
self->po_state = CHANGED;
*after* the call to _PyPersist_
Note, however, that if a data manager's register() should happen to modify the object, this will result in potentially unbounded recursion. This should be documented in the IPersistentData
I'd be more than happy to handle the necessary fixes and docs in CVS.
affects: | zope3 → zodb |
Also note that _PyPersist_ RegisterDataMan ager() itself sets
po_state to CHANGED even if register() raised an exception. It's somewhat questionable whether it has any business setting po_state at all; none of the places that call it inside cPersistence rely on it doing this; they all set po_state to CHANGED themselves.
It also appears as though there is considerable redundant code for state management surrounding the places that call _PyPersist_Load(); but the logic used in each case is slightly different. At this point, I'm not 100% positive I've caught all the places that suffer from this same logic error (i.e., leaving po_state CHANGED when an error occurs). :(