Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POSKeyError in Connection._commit_savepoint #402

Open
d-maurer opened this issue Oct 10, 2024 · 0 comments
Open

POSKeyError in Connection._commit_savepoint #402

d-maurer opened this issue Oct 10, 2024 · 0 comments

Comments

@d-maurer
Copy link
Contributor

PROBLEM REPORT

The problem was originally reported in plone/plone.restapi#1823 (comment)
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 "plone/Products.CMFPlone#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)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant