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

Test failure with Python 3.8.0 #1946

Closed
susilehtola opened this issue Oct 4, 2019 · 5 comments
Closed

Test failure with Python 3.8.0 #1946

susilehtola opened this issue Oct 4, 2019 · 5 comments

Comments

@susilehtola
Copy link

pybind11 version 2.4.2 fails to build from source in Fedora rawhide with Python 3.8.0rc1:

_____________________________ test_class_refcount ______________________________
    @pytest.unsupported_on_pypy
    def test_class_refcount():
        """Instances must correctly increase/decrease the reference count of their types (#1029)"""
        from sys import getrefcount
    
        class PyDog(m.Dog):
            pass
    
        for cls in m.Dog, PyDog:
            refcount_1 = getrefcount(cls)
            molly = [cls("Molly") for _ in range(10)]
            refcount_2 = getrefcount(cls)
    
            del molly
            pytest.gc_collect()
            refcount_3 = getrefcount(cls)
    
>           assert refcount_1 == refcount_3
E           assert 6 == 16
test_class.py:255: AssertionError

as reported in Fedora ticket https://bugzilla.redhat.com/show_bug.cgi?id=1758474

@wjakob
Copy link
Member

wjakob commented Oct 4, 2019

Oh oh, that's not good. It sounds very much like a regression in the Python beta version though. (Something is leaking references in Python, which is tricky to track down because it just wastes memory instead of e.g. causing a crash.)

@wjakob
Copy link
Member

wjakob commented Oct 4, 2019

I'm super-busy with a grant deadline in the next 2 weeks and unfortunately won't have free clock cycles to get to this-- it would be great if somebody could help out here. The task would be to git bisect Python master to the commit that caused this test to fail and then post a ticket on the Python bugtracker. (Assuming that my hunch about this being a Python regression is correct.)

@sizmailov
Copy link
Contributor

@wjakob It's not a bug, it's change in python3.8 C API

Instances of heap-allocated types (such as those created with PyType_FromSpec()) hold a reference to their type object. Increasing the reference count of these type objects has been moved from PyType_GenericAlloc() to the more low-level functions, PyObject_Init() and PyObject_INIT(). This makes types created through PyType_FromSpec() behave like other classes in managed code.
Statically allocated types are not affected.
For the vast majority of cases, there should be no side effect. However, types that manually increase the reference count after allocating an instance (perhaps to work around the bug) may now become immortal. To avoid this, these classes need to call Py_DECREF on the type object during instance deallocation.

https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-the-c-api

sizmailov added a commit to sizmailov/pybind11 that referenced this issue Oct 8, 2019
Do `Py_DECREF(type)` on all python objects on deallocation

fix pybind#1946
@wjakob wjakob closed this as completed in 6cb584e Oct 8, 2019
@wjakob
Copy link
Member

wjakob commented Oct 15, 2019

@susilehtola FYI: A new release of pybind11 (v2.4.3) is out. The Fedora package will need to be updated to this revision to ensure compatibility with Python 3.8.

@susilehtola
Copy link
Author

@wjakob thanks for the ping. I had already built 2.4.2 successfully with 6cb584e, but I've now bumped the package to 2.4.3.

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

3 participants