zopefoundation / persistent

automatic persistence for Python objects
https://pypi.org/project/persistent/
Other
46 stars 28 forks source link

feat: implement PEP489 (multi-phase module init, heap-allocated types) for Python >= 3.11 #204

Open tseaver opened 3 months ago

tseaver commented 3 months ago

Older Python versions continue to use static type init and static classes, although static state has been moved into the module state unconditionally.

I realize these changes are likely to be disruptive, in particular, for BTrees, which I plan to handle next.

davisagli commented 3 weeks ago

I tried running tox -e py311 using a Python built with --with-assertions. It fails to compile:

      building 'persistent.cPickleCache' extension
      clang -Wsign-compare -Wunreachable-code -g -fwrapv -O3 -Wall -I/Users/davisagli/Plone/persistent/.tox/py311/include -I/Users/davisagli/.pyenv/versions/3.11.8/include/python3.11 -c src/persistent/cPickleCache.c -o build/temp.macosx-13.6-arm64-cpython-311/src/persistent/cPickleCache.o
      src/persistent/cPickleCache.c:640:27: error: no member named 'ob_refcnt' in 'cPersistentObject'
          assert(dead_pers_obj->ob_refcnt == 0);
                 ~~~~~~~~~~~~~  ^
      /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/assert.h:99:25: note: expanded from macro 'assert'
          (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __ASSERT_FILE_NAME, __LINE__, #e) : (void)0)
                              ^
      src/persistent/cPickleCache.c:650:27: error: no member named 'ob_refcnt' in 'cPersistentObject'
          assert(dead_pers_obj->ob_refcnt == 1);
                 ~~~~~~~~~~~~~  ^
      /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/assert.h:99:25: note: expanded from macro 'assert'
          (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __ASSERT_FILE_NAME, __LINE__, #e) : (void)0)
                              ^
      src/persistent/cPickleCache.c:679:27: error: no member named 'ob_refcnt' in 'cPersistentObject'
          assert(dead_pers_obj->ob_refcnt == 1);
                 ~~~~~~~~~~~~~  ^
      /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/assert.h:99:25: note: expanded from macro 'assert'
          (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __ASSERT_FILE_NAME, __LINE__, #e) : (void)0)
                              ^
      3 errors generated.
      error: command '/usr/bin/clang' failed with exit code 1
tseaver commented 3 weeks ago

@davisagli 607cb29 fixes the compile-time errors under a Python built using --with-assert -- there are other issues at runtime I haven't chased down yet.

tseaver commented 3 weeks ago

@davisagli The last four commits clear up the remaining issues: tests now pass without segfaults under both "normal" Python >= 3.11 and under a --with-debug / --with-asserts Python.

tseaver commented 3 weeks ago

@davisagli re c8b34ca6:

I backed out the Py_INCREF (and related Py_VISIT and Py_CLEAR) in cPickleCache.c for the capi_struct->pertype member: the capi_struct is literally just a borrowed pointer to the structure which is part of the module state in cPersistence.c, and cPickleCache doesn't have the responsibility to manage it. The Py_CLEAR, in particular, could cause a segfault during normal interpreter shutdown.

tseaver commented 2 weeks ago

@davisagli

You mentioned that this will be disruptive. Should we bump the major version as part of merging this?

I'm not sure. Likely should try installing the wheel built from this branch into tox environments for the BTrees master branch, and see if anything breaks. AFAIK, BTrees is the only C-level consumer of the headers in persistent. Oops, no, the Zope2 Persistence lib uses it too.