classification
Title: `PyType_FromSpec` does not copy the name
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: petr.viktorin, seberg, shihai1991
Priority: normal Keywords: patch

Created on 2021-09-28 20:45 by seberg, last changed 2021-10-21 10:04 by petr.viktorin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29103 merged petr.viktorin, 2021-10-20 16:37
Messages (6)
msg402804 - (view) Author: Sebastian Berg (seberg) * Date: 2021-09-28 20:45
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:

https://github.com/python/cpython/blob/0c50b8c0b8274d54d6b71ed7bd21057d3642f138/Objects/typeobject.c#L3427

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.
msg403347 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2021-10-07 03:06
> 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:)
msg403734 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-10-12 13:49
> 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.)
msg404493 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-10-20 16:35
> 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.
msg404577 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-10-21 09:46
New changeset 8a310dd5f4242b5d28013c1905c6573477e3b957 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)
https://github.com/python/cpython/commit/8a310dd5f4242b5d28013c1905c6573477e3b957
msg404578 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2021-10-21 10:04
Since the fix changes the size of PyObject, it can't be backported.
History
Date User Action Args
2021-10-21 10:04:47petr.viktorinsetstatus: open -> closed
resolution: fixed
messages: + msg404578

stage: patch review -> resolved
2021-10-21 09:46:30petr.viktorinsetmessages: + msg404577
2021-10-20 16:37:57petr.viktorinsetkeywords: + patch
stage: patch review
pull_requests: + pull_request27371
2021-10-20 16:35:05petr.viktorinsetmessages: + msg404493
2021-10-12 13:49:36petr.viktorinsetmessages: + msg403734
2021-10-07 03:06:19shihai1991setnosy: + petr.viktorin, shihai1991
messages: + msg403347
2021-09-28 20:45:06sebergcreate