Memory mismanagement with Py_tp_doc

[loewis]

BPO	11249
Nosy	@loewis, @birkenfeld, @pitrou
Files	tp_doc.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2011-02-19.21:47:49.842>
created_at = <Date 2011-02-19.16:38:30.750>
labels = ['release-blocker']
title = 'Memory mismanagement with Py_tp_doc'
updated_at = <Date 2011-02-19.21:47:49.817>
user = 'https://github.com/loewis'

bugs.python.org fields:

activity = <Date 2011-02-19.21:47:49.817>
actor = 'georg.brandl'
assignee = 'none'
closed = True
closed_date = <Date 2011-02-19.21:47:49.842>
closer = 'georg.brandl'
components = []
creation = <Date 2011-02-19.16:38:30.750>
creator = 'loewis'
dependencies = []
files = ['20797']
hgrepos = []
issue_num = 11249
keywords = ['patch']
message_count = 12.0
messages = ['128857', '128858', '128859', '128860', '128863', '128871', '128872', '128873', '128879', '128880', '128881', '128882']
nosy_count = 3.0
nosy_names = ['loewis', 'georg.brandl', 'pitrou']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue11249'
versions = []

[loewis]

Currently, memory management for the Py_tp_doc slot in PyType_FromSpec is ill-defined. The doc string being passed is stored in the type object as-is, but later released with PyObject_Free. To make this consistent, PyType_FromSpec should copy the string, so that it's guaranteed that relasing the memory matches allocation.

Without this patch, users will have to hold onto the type objects they get from PyType_FromSpec "forever".

[pitrou]

Sure it should be a blocker?

[loewis]

If this isn't added, people using the API might see crashes. If they then work around crashes, they will see memory leaks in future releases.

[pitrou]

Well, isn't Py_tp_doc characteristically bound to a constant char *, so that the problem doesn't manifest itself?

[birkenfeld]

Sounds like a blocker to me. Martin, will you be able to provide a patch before final?

[loewis]

Well, isn't Py_tp_doc characteristically bound to a constant char *

That's exactly the problem: PyType_FromSpec puts in this string literal.
When the type is later released, PyObject_Free is called in
type_dealloc, which segfaults.

[loewis]

Sounds like a blocker to me. Martin, will you be able to provide a patch before final?

The attached patch works fine for me.

[pitrou]

> Sounds like a blocker to me. Martin, will you be able to provide a patch before final?

The attached patch works fine for me.

Is there some documentation somewhere for all this?

[loewis]

>> Sounds like a blocker to me. Martin, will you be able to provide a patch before final?
>
> The attached patch works fine for me.

Is there some documentation somewhere for all this?

Can you please be more specific? What's "all this"?

That tp_doc will be copied is currently undocumented. PyType_FromSpec
is documented in the PEP.

[pitrou]

>>> Sounds like a blocker to me. Martin, will you be able to provide a patch before final?
>>
>> The attached patch works fine for me.
>
> Is there some documentation somewhere for all this?

Can you please be more specific? What's "all this"?

That tp_doc will be copied is currently undocumented. PyType_FromSpec
is documented in the PEP.

I meant all of py_tp_doc, "limited api", "restricted api" show almost
zero hits in the API docs. A PEP is nice but I'm not sure many people
spontaneously read PEPs.

[loewis]

> That tp_doc will be copied is currently undocumented. PyType_FromSpec
> is documented in the PEP.

I meant all of py_tp_doc, "limited api", "restricted api" show almost
zero hits in the API docs. A PEP is nice but I'm not sure many people
spontaneously read PEPs.

No, there is no documentation in the Doc directory. I *would* expect
that people interested in using the stable ABI do find the PEP and
read it. With the exception of this issue, I consider the PEP to be
complete and correct documentation (of course, it probably contains
errors that I'm unaware of).

However, documentation for the PEP-384 changes seems to be an
unrelated issue.

[birkenfeld]

OK, committed in r88443 (with error handling made more consistent).

BTW, the return from PyType_GenericAlloc isn't NULL-checked, which looks like a potential crasher to me. Not release-critical though.