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 segfault when repr() with pybind11 type with metaclass #571

Merged
merged 4 commits into from
Dec 26, 2016

Conversation

yungyuc
Copy link
Contributor

@yungyuc yungyuc commented Dec 25, 2016

Correct the ht_qualname field for PyHeapTypeObject with metaclass.

When filling the ht_qualname field of the metaclass PyHeapTypeObject, the object shouldn't be released. Otherwise later the major type object receives a NULL ht_qualname, and then __repr__() results into segfault (typeobject.c:type_qualname() increments the ref count unconditionally by Py_INCREF(et->ht_qualname)).

Note that pickler relies on repr, and crashes as well. If needed I can provide a test case.

When filling the ht_qualname field of the metaclass PyHeapTypeObject, the object shouldn't be released.  Otherwise later the major type object receives a NULL ht_qualname, and then __repr__() results into segfault (typeobject.c:type_qualname() increment the ref count unconditionally by Py_INCREF(et->ht_qualname)).

Note that pickler relies on repr, and crashes as well.
@wjakob
Copy link
Member

wjakob commented Dec 25, 2016

I think the issue here is actually that it should read

ht_qualname_meta.release().ptr()

instead of

ht_qualname.release().ptr()

(introduced by accident with the PyPy patch). Could you adapt the PR accordingly?

Thanks,
Wenzel

…ht_qualname field.

Change 806b677 incorrectly inc_ref'ed ht_qualname, which left ht_qualname_meta remains unused.
@yungyuc
Copy link
Contributor Author

yungyuc commented Dec 26, 2016

That's right. I overlooked it. Thanks for pointing it out, @wjakob.

Please let me know if the multiple checkins should be be combined into one.

@wjakob wjakob merged commit c40d8c6 into pybind:master Dec 26, 2016
@wjakob
Copy link
Member

wjakob commented Dec 26, 2016

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

Successfully merging this pull request may close these issues.

None yet

2 participants