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

gh-94673: Ensure Builtin Static Types are Readied Properly #103940

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Apr 27, 2023

There were cases where we do unnecessary work for builtin static types. This also simplifies some work necessary for a per-interpreter GIL.

@ericsnowcurrently ericsnowcurrently merged commit d2e2e53 into python:main Apr 27, 2023
@ericsnowcurrently ericsnowcurrently deleted the ensure-builtin-static-type-ready branch April 27, 2023 22:19
ericsnowcurrently added a commit that referenced this pull request Apr 28, 2023
…3961)

This change has two small parts:

1. a follow-up to gh-103940 with one case I missed
2. adding a missing return that I noticed while working on related code
@Forty-Bot
Copy link

This causes a regression for some users of PyModule_AddType. Previously, you could do something like

PyTypeObject MyType = {
	PyObject_HEAD_INIT(NULL)
	/* ... */
};

int MyType_Add(PyObject *m)
{
	int err;
	PyObject *obj = PyLong_FromDouble(1.0);

	if (!obj)
		return -1;

	MyType.tp_dict = PyDict_New();
	if (!type->tp_dict) {
		Py_DECREF(obj);
		return -1;
	}

	err = PyDict_SetItemString(MyType.tp_dict, "foo", obj);
	Py_DECREF(obj);
	if (err)
		return err;

	if (PyModule_AddType(m, &MyType))
		return -1;

	return 0;
}

but now this will now segfault, as PyModule_AddType will never call PyType_Ready since tp_dict is initialized. The documentation for PyModule_AddType says:

Add a type object to module. The type object is finalized by calling internally PyType_Ready().

with no mention of tp_dict. On the subject of when to call PyType_Ready, the documentation for PyObject.ob_type says:

Therefore, the convention is to pass NULL to the PyObject_HEAD_INIT macro and to initialize this field explicitly at the start of the module’s initialization function, before doing anything else...

This should be done before any instances of the type are created.

PyTypeObject.tp_dict may also be initialized before calling PyType_Ready:

This field should normally be initialized to NULL before PyType_Ready is called; it may also be initialized to a dictionary containing initial attributes for the type.

As PyModule_AddType is part of the stable ABI, I believe that using _PyType_IsReady in PyModule_AddType should be reverted.

Forty-Bot added a commit to Forty-Bot/mpmetrics that referenced this pull request Nov 19, 2023
Workaround for [1].

[1] python/cpython#103940

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants