Description

colesbury

opened on Jan 5, 2024

Issue body actions

Feature or enhancement

The MRO (method resolution order) cache is used to cache lookups on the type hierarchy, such as method lookups. It's currently implemented per-interpreter, which will not be thread-safe in free-threaded builds.

cpython/Objects/typeobject.c

Lines 4750 to 4803 in 5e1916b

	/* Internal API to look for a name through the MRO.
	This returns a borrowed reference, and doesn't set an exception! */
	PyObject *
	_PyType_Lookup(PyTypeObject *type, PyObject *name)
	{
	PyObject *res;
	int error;
	PyInterpreterState *interp = _PyInterpreterState_GET();
	unsigned int h = MCACHE_HASH_METHOD(type, name);
	struct type_cache *cache = get_type_cache();
	struct type_cache_entry *entry = &cache->hashtable[h];
	if (entry->version == type->tp_version_tag &&
	entry->name == name) {
	assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG));
	OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name));
	OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name));
	return entry->value;
	}
	OBJECT_STAT_INC_COND(type_cache_misses, !is_dunder_name(name));
	OBJECT_STAT_INC_COND(type_cache_dunder_misses, is_dunder_name(name));
	/* We may end up clearing live exceptions below, so make sure it's ours. */
	assert(!PyErr_Occurred());
	res = find_name_in_mro(type, name, &error);
	/* Only put NULL results into cache if there was no error. */
	if (error) {
	/* It's not ideal to clear the error condition,
	but this function is documented as not setting
	an exception, and I don't want to change that.
	E.g., when PyType_Ready() can't proceed, it won't
	set the "ready" flag, so future attempts to ready
	the same type will call it again -- hopefully
	in a context that propagates the exception out.
	*/
	if (error == -1) {
	PyErr_Clear();
	}
	return NULL;
	}
	if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(interp, type)) {
	h = MCACHE_HASH_METHOD(type, name);
	struct type_cache_entry *entry = &cache->hashtable[h];
	entry->version = type->tp_version_tag;
	entry->value = res; /* borrowed */
	assert(_PyASCIIObject_CAST(name)->hash != -1);
	OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
	assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG));
	Py_SETREF(entry->name, Py_NewRef(name));
	}
	return res;
	}

The nogil-3.9 and nogil-3.12 forks used different approaches to make the MRO cache thread-safe. The nogil-3.9 fork moved the type cache to the PyThreadState. The nogil-3.12 fork implemented a thread-safe cache shared across threads ¹. The nogil-3.9 approach is much simpler, I think we should start with that.

Suggested approach:

• If Py_GIL_DISABLED is defined, move the type_cache to the private struct PyThreadStateImpl.
• Refactor _PyType_Lookup() as necessary to use the correct cache

Linked PRs

• gh-113743: Make the MRO cache thread-safe in free-threaded builds #113930
• gh-113743: Use per-interpreter locks for types #115541
• gh-113743: Give _PyTypes_AfterFork a prototype. #115563

Footnotes

1. For reference, here is the nogil-3.12 implementation: https://github.com/colesbury/nogil-3.12/commit/9c1f7ba1b4 ↩

Activity

1. colesbury

   added

   type-featureA feature request or enhancementA feature request or enhancement

   interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)

   3.13only security fixesonly security fixes

   topic-free-threading

   on Jan 5, 2024
2. DinoV

   self-assigned this

   on Jan 9, 2024
3. DinoV

   added 2 commits that reference this issue on Feb 15, 2024

   gh-113743: Make the MRO cache thread-safe in free-threaded builds (#1…

   ae460d4

   gh-113743: Use per-interpreter locks for types (#115541)

   454d796
4. benjaminp

   added 2 commits that reference this issue on Feb 16, 2024

   pythongh-113743: Give _PyTypes_AfterFork a prototype.

   98dc3e7

   gh-113743: Give _PyTypes_AfterFork a prototype. (gh-115563)

   2ac9d9f
5. DinoV

   closed this as completedon Feb 29, 2024
6. woodruffw

   added a commit that references this issue on Mar 4, 2024

   pythongh-113743: Give _PyTypes_AfterFork a prototype. (pythongh-115563)

   e1fa6cd
7. diegorusso

   added 3 commits that reference this issue on Apr 17, 2024

   pythongh-113743: Make the MRO cache thread-safe in free-threaded buil…

   fe1f3e3

   pythongh-113743: Use per-interpreter locks for types (python#115541)

   74444d5

   pythongh-113743: Give _PyTypes_AfterFork a prototype. (pythongh-115563)

   ecb60f7
8. LukasWoodtli

   added 3 commits that reference this issue on Jan 22, 2025

   pythongh-113743: Make the MRO cache thread-safe in free-threaded buil…

   43ac01e

   pythongh-113743: Use per-interpreter locks for types (python#115541)

   f6b6f95

   pythongh-113743: Give _PyTypes_AfterFork a prototype. (pythongh-115563)

   ea9d255

Issue actions