zopefoundation / ZODB

Python object-oriented database
https://zodb-docs.readthedocs.io/
Other
682 stars 92 forks source link

`POSKeyError` in `Connection._commit_savepoint` #402

Open d-maurer opened 1 month ago

d-maurer commented 1 month ago

PROBLEM REPORT

The problem was originally reported in https://github.com/plone/plone.restapi/pull/1823#issuecomment-2399816257 but can happen in other (rare) contexts, too.

Connection implements savepoints via a storage stack. At its bottom is the "normal storage". To handle a savepoint, a "temp storage" is pushed onto this stack. The "temp storage" contains the state changed by the savepoint. Connection._commit_savepoint starts by popping the "temp storage" from the stack, as a consequence the changes can no longer be accessed (via the standard ZODB API).

In the error case mentioned above, the savepoint has created new objects, one of them (a PloneSite) with an unfortunate __setattr__ definition (see "https://github.com/plone/Products.CMFPlone/pull/4026") accessing an object created by the savepoint and no longer in the ZODB object cache. Loading this object from the ZODB failed with the POSKeyError because the savepoint changes were no longer accessible. The __setattr__ was called for the a _p_estimated_size update in _commit_savepoint.

Likely, the problem can only occur when an object with a custom __setattr__ is involved which accesses persistent objects even though a special persistent attribute (like _p_estimated_size) is updated. One can consider such a __setattr__ as buggy. We nevertheless might want to fix the problem in _commit_savepoint. Likely, it is sufficient to perform the "temp storage" pop at the end (rather than the start) of _commit_savepoint (this would require to adapt the self._storage references in _commit_savepoint (self._storage --> self._normal_storage)).