"REGISTER" test fails against pure Python version of `collections.OrderedDict`

[mgorny]

Python stdlib has two versions of collections.OrderedDict:

• an optimized C version from the built-in _collections module that is used by default: https://github.com/python/cpython/blob/4b15d105a2b954cce2d97646ebf9e672d3a23b45/Lib/collections/__init__.py#L346
• a fallback pure Python implementation: https://github.com/python/cpython/blob/4b15d105a2b954cce2d97646ebf9e672d3a23b45/Lib/collections/__init__.py#L65-L342

Similarly, PyPy has an optimized _collections module written in RPython. Unfortunately, this module turned out to be quite broken in a way that upstream is still working on figuring out (pytest-dev/pytest#13312). To fix the immediate issue and resolve breakage for end users, in Gentoo we have reverted to using the pure Python version (https://gitweb.gentoo.org/fork/pypy.git/commit/?h=gentoo-3.11-7.3.19_p6&id=5fc7dcfbfb59e2c5c9614a6d93019bc1c9dc7ada). However, this seems to break dill, presumably because it doesn't implement serialization for the respective pure Python types:

$ tox -e pypy310,pypy311
.pkg: _optional_hooks> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_wheel> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: prepare_metadata_for_build_wheel> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_sdist> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
pypy310: install_package> python -I -m pip install --force-reinstall --no-deps /tmp/dill/.tox/.tmp/package/5/dill-0.4.1.dev0.tar.gz
pypy310: commands[0]> .tox/pypy310/bin/pypy3 -m pip install .
Processing /tmp/dill
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: dill
  Building wheel for dill (pyproject.toml) ... done
  Created wheel for dill: filename=dill-0.4.1.dev0-py3-none-any.whl size=119871 sha256=a2327bc1b76bb91c857182056117114cbc99c4b642cbe4e04bb2c1fdfe3ca8a2
  Stored in directory: /tmp/pip-ephem-wheel-cache-15t0mqp_/wheels/8f/c2/ef/34e11f15a145dc577fdd777792c27173d1aae4dd9fa7b5f843
Successfully built dill
Installing collected packages: dill
  Attempting uninstall: dill
    Found existing installation: dill 0.4.1.dev0
    Uninstalling dill-0.4.1.dev0:
      Successfully uninstalled dill-0.4.1.dev0
Successfully installed dill-0.4.1.dev0
pypy310: commands[1]> .tox/pypy310/bin/pypy3 dill/tests/__main__.py
........REGISTER: ['OdictValuesType']
Traceback (most recent call last):
  File "/tmp/dill/dill/tests/test_registered.py", line 38, in <module>
    raise e from None
  File "/tmp/dill/dill/tests/test_registered.py", line 35, in <module>
    assert not bool(register)
AssertionError
F...................<function <lambda> at 0x00007f9c055da700>
<function <lambda> at 0x00007f023dcecf20>
<function <lambda> at 0x00007f560b0ecf20>
<function <lambda> at 0x00007fc6b3ceac00>
<function <lambda> at 0x00007f278426d060>
..
pypy310: exit 1 (7.51 seconds) /tmp/dill> .tox/pypy310/bin/pypy3 dill/tests/__main__.py pid=53535
pypy310: FAIL ✖ in 16.34 seconds
pypy311: install_package> python -I -m pip install --force-reinstall --no-deps /tmp/dill/.tox/.tmp/package/6/dill-0.4.1.dev0.tar.gz
pypy311: commands[0]> .tox/pypy311/bin/pypy3 -m pip install .
Processing /tmp/dill
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: dill
  Building wheel for dill (pyproject.toml) ... done
  Created wheel for dill: filename=dill-0.4.1.dev0-py3-none-any.whl size=119871 sha256=a6dbb92ddc8004dcaff1c7bda2d7b5e94c2a5b608677e8ada7c9fd63f1d022e7
  Stored in directory: /tmp/pip-ephem-wheel-cache-3nskesmc/wheels/86/1c/89/9baf451a87f3342f49287a63390bcdb2d774fb063bc7ed476c
Successfully built dill
Installing collected packages: dill
  Attempting uninstall: dill
    Found existing installation: dill 0.4.1.dev0
    Uninstalling dill-0.4.1.dev0:
      Successfully uninstalled dill-0.4.1.dev0
Successfully installed dill-0.4.1.dev0
pypy311: commands[1]> .tox/pypy311/bin/pypy3 dill/tests/__main__.py
........REGISTER: ['OdictValuesType']
Traceback (most recent call last):
  File "/tmp/dill/dill/tests/test_registered.py", line 38, in <module>
    raise e from None
  File "/tmp/dill/dill/tests/test_registered.py", line 35, in <module>
    assert not bool(register)
AssertionError
F...................<function <lambda> at 0x00007fdb69f73ec0>
<function <lambda> at 0x00007f0d8f9d7ce0>
<function <lambda> at 0x00007f88019de340>
<function <lambda> at 0x00007f453e95e2a0>
<function <lambda> at 0x00007f1c51259920>
..
pypy311: exit 1 (7.64 seconds) /tmp/dill> .tox/pypy311/bin/pypy3 dill/tests/__main__.py pid=54033
  pypy310: FAIL code 1 (16.34=setup[4.90]+cmd[3.93,7.51] seconds)
  pypy311: FAIL code 1 (15.40=setup[3.93]+cmd[3.83,7.64] seconds)
  evaluation failed :( (31.81 seconds)

The issue can be reproduced by disabling the from _collections import OrderedDict import in collections/__init__.py.

Curious enough, with CPython the incompatibility seems even worse. If I disable the import there, I get:

$ tox -e py310
.pkg: _optional_hooks> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_wheel> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: prepare_metadata_for_build_wheel> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: build_sdist> python /usr/lib/python3.13/site-packages/pyproject_api/_backend.py True setuptools.build_meta
py310: install_package> python -I -m pip install --force-reinstall --no-deps /tmp/dill/.tox/.tmp/package/3/dill-0.4.1.dev0.tar.gz
py310: commands[0]> .tox/py310/bin/python -m pip install .
Processing /tmp/dill
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: dill
  Building wheel for dill (pyproject.toml) ... done
  Created wheel for dill: filename=dill-0.4.1.dev0-py3-none-any.whl size=119871 sha256=faba13f99dede7e9ad0a2a0195cbd27681732d3174507f25563d15eac30f10fc
  Stored in directory: /tmp/pip-ephem-wheel-cache-0ak6lua3/wheels/ba/13/46/41a6e38bc5d4fae7b5d931dab0455e651d13abbde68423fbb2
Successfully built dill
Installing collected packages: dill
  Attempting uninstall: dill
    Found existing installation: dill 0.4.1.dev0
    Uninstalling dill-0.4.1.dev0:
      Successfully uninstalled dill-0.4.1.dev0
Successfully installed dill-0.4.1.dev0
py310: commands[1]> .tox/py310/bin/python dill/tests/__main__.py
........REGISTER: ['OdictKeysType', 'OdictValuesType', 'OdictItemsType']
Traceback (most recent call last):
  File "/tmp/dill/dill/tests/test_registered.py", line 38, in <module>
    raise e from None
  File "/tmp/dill/dill/tests/test_registered.py", line 35, in <module>
    assert not bool(register)
AssertionError
F...................<function <lambda> at 0x7f09d9763d90>
<function <lambda> at 0x7f30bdd63d90>
<function <lambda> at 0x7f6909963d90>
<function <lambda> at 0x7fb526f63d90>
<function <lambda> at 0x7f82bb567d90>
..
py310: exit 1 (2.54 seconds) /tmp/dill> .tox/py310/bin/python dill/tests/__main__.py pid=52093
  py310: FAIL code 1 (8.27=setup[3.75]+cmd[1.97,2.54] seconds)
  evaluation failed :( (8.34 seconds)

[mmckerns]

I need to understand better what you are doing, but at first glance it seems that you are overriding an import to use the pure python version instead of the C implementation. If that's the case, the dill test that is failing is because it's testing the expected behavior of OrderedDict. Essentially, the test confirms objects expected to fail to serialize still fail, objects expected serialize still succeed, etc. For ['OdictKeysType', 'OdictValuesType', 'OdictItemsType'], dill has to register these types in the pickle registry for them to be serializable, so they are tested even further for unexpected changes. The tracebacks are just telling me that the expected serialization behavior changed, not that the objects fail to serialize.

I believe the assertion errors from the test can by first identifying if the OrderedDict is the python or C version, and then if it's the pure python version adding the above types to the relevant "expected to serialize" category. So, it's not breaking dill per se, it just telling me the expected behavior has changed.

What do you propose to be done?

I think a sufficient check is that collections.OrderedDict is _collections.OrderedDict, and then register the relevant types to the pickle registry (or not). However, I don't know what impact this would have on downstream packages that depend on stored serialized objects not changing.

[mgorny]

I think a sufficient check is that collections.OrderedDict is _collections.OrderedDict, and then register the relevant types to the pickle registry (or not).

Yes, I think that would work.

However, I don't know what impact this would have on downstream packages that depend on stored serialized objects not changing.

Yeah, I wish we wouldn't have to do this, but at this point I think the harm from collections.OrderedDict randomly corrupting the data is worse than the risk of serialization changes. And given how tricky the issue is, we don't have any ETA on a proper fix — or even if there is a point in "proper" fix, given how efficient PyPy is in executing pure Python code.

[mmckerns]

I just checked the serialization of all relevant objects for the pure python version of OrderedDict works as expected, and a straightforward workaround that could be added to dill is exactly what I suggested. This part should be trivial, and cause no impact for newly serialized objects.

The risk is that it breaks key infrastructure in the ecosystem that has stored OrderedDict objects from _collections. To ensure that doesn't happen is much more complicated. I think one would have to register a serialization method for the pure python types which check if the pure python version of OrderDict is the only available one, and then if so, convert the stored non-python OrderedDict object to a pure python equivalent. That's non-trivial and requires a modified and un-modified version of python to test. This part will require some thought.

[mgorny]

For the record, I think only the serialization of OrderedDict.values() objects is affected here. OrderedDict itself seems to be serialized the same across CPython and both PyPy variants, at least according to a quick experiment.

[mmckerns]

Probably the best way to compare is turn on dill.detect.trace(True).

For the record, I think only the serialization of OrderedDict.values() objects is affected here.

I think only new OrderedDict.values() are affected. The issue is that stored objects using the definition in _collections need to be able to be loaded into the pure python versions. I'll do some testing.

[mgorny]

Thank you for all the effort you're putting into this! It's very much appreciated.

[mmckerns]

I was able to unpickle a stored instances of a non-python OrderedDict into a pure-python OrderedDict, and similarly for the associated values, keys, etc. So, there should be no issues with converting stored instances.