Skip to content

Segfault with shared_ptr holder and def_readwrite #543

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

Closed
bennybp opened this issue Dec 6, 2016 · 3 comments
Closed

Segfault with shared_ptr holder and def_readwrite #543

bennybp opened this issue Dec 6, 2016 · 3 comments

Comments

@bennybp
Copy link
Contributor

bennybp commented Dec 6, 2016

The following MWE produces a segfault. This isn't compiler or optimization specific. The segfault goes away if the holder type is removed from TestA. Valgrind shows that it is an invalid delete/free.

CPP file:

#include <pybind11/pybind11.h>
#include <iostream>
#include <memory>

PYBIND11_DECLARE_HOLDER_TYPE(T__,std::shared_ptr<T__>)

struct TestA{
    TestA() = default;
    int x;
};

struct TestB{
    TestB() = default;
    TestA myA;
};

PYBIND11_PLUGIN(mwe) {
    pybind11::module m("mwe");

    pybind11::class_<TestA, std::shared_ptr<TestA>>(m,"TestA")
    .def(pybind11::init<>())
    .def_readwrite("x",&TestA::x)
    ;

    pybind11::class_<TestB>(m,"TestB")
    .def(pybind11::init<>())
    .def_readwrite("myA",&TestB::myA);

    return m.ptr();
}

Python script:

from mwe import *

a = TestA()
a.x = 2
b = TestB()
b.myA = a
print(b.myA)

git bisect shows the error introduced with commit bd560ac

I'm thinking that when constructing from another holder, it shouldn't be owned by the new object?

@dean0x7d
Copy link
Member

dean0x7d commented Dec 6, 2016

This should be fixed in PR #533 which is pending a small revision -- I will address this shortly.

@jagerman
Copy link
Member

jagerman commented Dec 6, 2016

(I just verified that it is indeed fixed by #533).

@bennybp
Copy link
Contributor Author

bennybp commented Dec 8, 2016

This does seem fixed now that it has been merged. Thanks!

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