From aacb0f62b034fe5b6b26582e5f864d688c208756 Mon Sep 17 00:00:00 2001 From: Thomas Wouters Date: Wed, 9 Nov 2022 16:41:39 +0100 Subject: [PATCH 1/2] Fix stale method caches and assertion errors in SWIG-generated extension modules. Extension modules generated by SWIG up to version 4.1.0 clear the Py_TPFLAGS_VALID_VERSION_TAG bit from tp_flags when modifying the type, as they should, but do not update tp_version_tag as typeobject.c expects. (For example, merely one of many instances I've found: https://github.com/OSGeo/gdal/blob/6bd07b20b3e55c2fc94da611244a615a4fd2991f/swig/python/extensions/osr_wrap.cpp#L2296) In release builds this means a potentially stale cached entry is used, and in debug builds (or release builds that happen to enable assertions, which as it turns out is actually a good idea) it triggers an assertion error. SWIG 4.1.0 avoids this problem by using PyType_Modified(), but there are a lot of older versions of SWIG and of checked-in generated extension modules around. --- Objects/typeobject.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e2e40b5c15297d..d0f241d5b7c40b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4176,12 +4176,17 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) unsigned int h = MCACHE_HASH_METHOD(type, name); struct type_cache *cache = get_type_cache(); struct type_cache_entry *entry = &cache->hashtable[h]; + /* Some extension modules (e.g. those generated by SWIG) don't always + * change/clear tp_version_tag when they clear the + * Py_TPFLAGS_VALID_VERSION_TAG tp_flag, so check both tp_version_tag + * and the tp_flag. (Invalidated cache entries are replaced below.) + */ if (entry->version == type->tp_version_tag && - entry->name == name) { + entry->name == name && + _PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { #if MCACHE_STATS cache->hits++; #endif - assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)); return entry->value; } From 0e8b65d20e9deae630b4d1ca45f46fddc96d9f61 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 9 Nov 2022 16:02:31 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-11-09-16-02-27.gh-issue-99293.oPstNV.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-09-16-02-27.gh-issue-99293.oPstNV.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-09-16-02-27.gh-issue-99293.oPstNV.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-09-16-02-27.gh-issue-99293.oPstNV.rst new file mode 100644 index 00000000000000..730e1a79c407ac --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-09-16-02-27.gh-issue-99293.oPstNV.rst @@ -0,0 +1 @@ +Fix an issue where extension modules could inadvertently trigger an assertion error in typeobject.c by clearing the Py_TPFLAGS_VALID_VERSION_TAG tp_flag without clearing the tp_version_tag field. (Extension modules should use PyType_Modified() instead, however.)