`PyType_FromSpec` does not copy the name

[seberg]

BPO	45315
Nosy	@encukou, @seberg, @shihai1991
PRs	bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name #29103

^{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 2021-10-21.10:04:47.123>
created_at = <Date 2021-09-28.20:45:06.803>
labels = ['interpreter-core', 'type-crash', '3.11']
title = '`PyType_FromSpec` does not copy the name'
updated_at = <Date 2021-10-21.10:04:47.122>
user = 'https://github.com/seberg'

bugs.python.org fields:

activity = <Date 2021-10-21.10:04:47.122>
actor = 'petr.viktorin'
assignee = 'none'
closed = True
closed_date = <Date 2021-10-21.10:04:47.123>
closer = 'petr.viktorin'
components = ['Interpreter Core']
creation = <Date 2021-09-28.20:45:06.803>
creator = 'seberg'
dependencies = []
files = []
hgrepos = []
issue_num = 45315
keywords = ['patch']
message_count = 6.0
messages = ['402804', '403347', '403734', '404493', '404577', '404578']
nosy_count = 3.0
nosy_names = ['petr.viktorin', 'seberg', 'shihai1991']
pr_nums = ['29103']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue45315'
versions = ['Python 3.11']

[seberg]

As noted in the issue: https://bugs.python.org/issue15870#msg402800 PyType_FromSpec assumes that the name passed is persistent for the program lifetime. This seems wrong/unnecessary: We are creating a heap-type, a heap-type's name is stored as a unicode object anyway!

So, there is no reason to require the string be persistent, and a program that decides to build a custom name dynamically (for whatever reasons) would run into a crash when the name is accessed.

The code:

cpython/Objects/typeobject.c

Line 3427 in 0c50b8c

type->tp_name = spec->name;

Should be modified to point to to the ht_name (utf8) data instead. My guess is, the simplest solution is calling type_set_name, even if that runs some unnecessary checks.

I understand that the FromSpec API is mostly designed to replace the static type definition rather than extend it, but it seems an unintentional/strange requirement.

[shihai1991]

the simplest solution is calling type_set_name, even if that runs some unnecessary checks.

Hm, I haven't find any case who use dynamical tp_name of Type_Spec temporarily.
If we meet this user case, Adding a new object pointer of type name in is an other option maybe:)

[encukou]

Hm, I haven't find any case who use dynamical tp_name of Type_Spec temporarily.

The use case is creating types entirely on demand, with dynamically created specs.
This is useful e.g. for wrapping objects of a different object system, like C++ classes or Qt/GTK widgets.

But this issue is not about adding new API to enable a new use case. The *current* API can reasonably be used with a dynamic name string. So we shouldn't wait until someone tells us they need this fixed :)

So this should either be fixed or the requirement should be documented. (And the documentation would IMO sound like we're acknowledging a design bug, so it's better to fix.)

[encukou]

the simplest solution is calling type_set_name, even if that runs some unnecessary checks.

Unfortunately this won't work, because it sets ht_name to the same value as tp_name. For historical reasons, the two can be different (and often are) for types created with PyType_From*Spec*.

I haven't found a way to fix this issue without adding yet another pointer to the PyHeapTypeObject struct.

[encukou]

New changeset 8a310dd by Petr Viktorin in branch 'main':
bpo-45315: PyType_FromSpec: Copy spec->name and have the type own the memory for its name (GH-29103)
8a310dd

[encukou]

Since the fix changes the size of PyObject, it can't be backported.