Skip to content

Conversation

@mamico
Copy link
Member

@mamico mamico commented Jun 8, 2025

Fix: #218

IMHO in python 3.14 dictionary and/or garbage optimization requires to explicitly clear the dictionary.

Although this change seems to fix the tests in python 3.14 (tested with python 3.14.0b2), I suggest double-checking the change with someone more familiar with the internals of the library to exclude any side effects related to the change

@mamico mamico requested review from dataflake and jamadden June 8, 2025 23:08
@dataflake
Copy link
Member

I ran the latest zope.meta configuration so that the addition of Python 3.14 is reflected everywhere instead of just the GHA version specification. I have no idea why the pypy-3.10 test on Windows is failing while looking for setuptools 78.1.1, that makes no sense.

@dataflake
Copy link
Member

After reverting the setuptools version change the tests are turning green again. So there's yet another setuptools issue.

What confuses me about the PyPy build (as a non-expert) is that the C extensions are built and that the build instructions in the cffi_modules argument to setup in setup.py are run, even though cffi is only a dependency for CPython. I thought PyPy code doesn't build or use the C extensions?

Copy link
Member

@dataflake dataflake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setuptools issue is a separate problem, the dictionary fix should go ahead. I can't judge if that dictionary clearing has unwanted side effects, though.

@davisagli
Copy link
Member

davisagli commented Jun 16, 2025

Before this change, you can assign obj.__dict__ to another variable, then call obj._p_deactivate(), and still access the dict items via the other variable. After the change, the dict will be empty even if it's accessed via another reference. I'm not sure if that will cause problems but it seems worth noting.

@jamadden
Copy link
Member

Before this change, you can assign obj.__dict__ to another variable, then call obj._p_deactivate(), and still access the dict items via the other variable. After the change, the dict will be empty even if it's accessed via another reference. I'm not sure if that will cause problems but it seems worth noting.

This is how the pure-Python implementation already works, so I believe that should be the expected behaviour:

try:
idict = _OGA(self, '__dict__')
except AttributeError:
pass
else:
idict.clear()

Relstorage has tests that fail without this change, and which pass with this change and pass in PURE_PYTHON=1. The entire RelStorage test suite passes with this change.

+1 from me, and for getting this merged and released.

jamadden added a commit to zodb/relstorage that referenced this pull request Sep 29, 2025
@dataflake dataflake merged commit 813134b into master Sep 29, 2025
55 checks passed
@dataflake dataflake deleted the clear_dict_py314 branch September 29, 2025 15:49
@icemac
Copy link
Member

icemac commented Oct 2, 2025

Just released in https://pypi.org/project/persistent/6.2/.

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

Successfully merging this pull request may close these issues.

persistent: Unit tests fail on Python 3.14

6 participants