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

Fix deallocation of incorrect pointer (#1568) #1592

Closed
wants to merge 2 commits into from

Conversation

Blistic
Copy link
Contributor

@Blistic Blistic commented Nov 4, 2018

Fix #1568

@wjakob
Copy link
Member

wjakob commented Nov 9, 2018

Just a heads-up that this will take some time to investigate/merge. The multiple inheritance parts of pybind11 are very complex and took a long time to get right. I want to make sure that we don't cause a regression (that may not be tested by the test suite right now).

@Blistic
Copy link
Contributor Author

Blistic commented Nov 10, 2018

Yes of course, I do not fully understand the intricacies myself. This PR was mainly intended as a naive starting point for #1568 and for those who cannot restructure the code to simply avoid the issue. I will continue using this PR in my own work and if I come across any regressions I will update this thread accordingly. Thanks for the reply!

@eacousineau
Copy link
Contributor

eacousineau commented Mar 11, 2019

If there's any way I can help this land (e.g. combing through open-source uses of pybind11 + adding tests here to reflect those, adding / running benchmarks), I'd be happy to help :)

Ran into this with unique_ptr stuff in #1237.

@henryiii
Copy link
Collaborator

henryiii commented Oct 9, 2020

Alternate PR merged! Thanks!

@henryiii henryiii closed this Oct 9, 2020
@YannickJadoul
Copy link
Collaborator

Thanks again for your work and arguments on why this was the correct way of resolving it (in #1568 (comment)), @Blistic! (You've been included as co-author on the merged PR that solved the issue, as well :-) )

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.

Having a class which holds the same pointer inside can crash
5 participants