From 10399f7e156f8e69ac9b7aa960a34419ae93e2d2 Mon Sep 17 00:00:00 2001 From: chaerrypick01 Date: Mon, 17 Aug 2026 11:36:15 +0900 Subject: [PATCH 1/5] gh-105250: Fix NEWOBJ handling of custom metaclasses in C pickle The NEWOBJ and NEWOBJ_EX opcodes are documented to call cls.__new__(cls, *args), but the C implementation called tp_new directly, so a metaclass __getattribute__ hook was skipped unless the class happened to define __new__ in Python. Perform a real attribute lookup of cls.__new__ when the class has a custom metaclass, matching the pure Python implementation. The default-metaclass case keeps calling tp_new directly, where the lookup is not observable. --- Lib/test/picklecommon.py | 34 +++++++++++++++++++ Lib/test/pickletester.py | 15 ++++++++ ...-08-17-11-35-48.gh-issue-105250.wNaXOE.rst | 5 +++ Modules/_pickle.c | 31 ++++++++++++++++- 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst diff --git a/Lib/test/picklecommon.py b/Lib/test/picklecommon.py index 5dd56c4fbf9ec81..a0513731d63ac63 100644 --- a/Lib/test/picklecommon.py +++ b/Lib/test/picklecommon.py @@ -290,6 +290,40 @@ class MyIntWithNew2(MyIntWithNew): __new__ = int.__new__ +# For test_newobj_metaclass_lookup +metaclass_new_lookups = [] + +class LookupLoggingMeta(type): + def __getattribute__(cls, name): + if name == '__new__': + metaclass_new_lookups.append(name) + return super().__getattribute__(name) + +class NewInheriting(metaclass=LookupLoggingMeta): + # __new__ is inherited from object, so tp_new is a C slot function. + def __init__(self, value=None): + self.value = value + +class NewDefining(metaclass=LookupLoggingMeta): + # __new__ is defined in Python, so tp_new is slot_tp_new, which + # performs its own __new__ lookup through the metaclass. + def __new__(cls, *args): + return super().__new__(cls) + + def __init__(self, value=None): + self.value = value + +class NewInheritingEx(dict, metaclass=LookupLoggingMeta): + # Like NewInheriting, but pickled with NEWOBJ_EX (protocol 4+): + # non-empty kwargs force the NEWOBJ_EX opcode, and dict's C tp_new + # accepts them while __new__ stays inherited. + def __init__(self, value=None): + self.value = value + + def __getnewargs_ex__(self): + return (), {'ignored': True} + + # For test_newobj_list_slots class SlotList(MyList): __slots__ = ["foo"] diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c53262e358b48ea..c426d0f73653eda 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3599,6 +3599,21 @@ def test_newobj_overridden_new(self): self.assertEqual(int(y), 1) self.assertEqual(y.foo, 42) + def test_newobj_metaclass_lookup(self): + # gh-105250: NEWOBJ and NEWOBJ_EX look __new__ up on the class, + # so a custom metaclass __getattribute__ hook observes the lookup. + for cls, min_proto in ((NewInheriting, 2), + (NewDefining, 2), + (NewInheritingEx, 4)): + for proto in protocols[min_proto:]: + with self.subTest(cls=cls, proto=proto): + s = self.dumps(cls(42), proto) + metaclass_new_lookups.clear() + y = self.loads(s) + self.assertIs(type(y), cls) + self.assertEqual(y.value, 42) + self.assertIn('__new__', metaclass_new_lookups) + def test_newobj_not_class(self): # Issue 24552 if self.py_version < (3, 4): diff --git a/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst b/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst new file mode 100644 index 000000000000000..b5cf3c97034a995 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst @@ -0,0 +1,5 @@ +Fix a difference in behavior between the C and pure Python implementations of +:mod:`pickle` when unpickling with the ``NEWOBJ`` and ``NEWOBJ_EX`` opcodes: +the C implementation now performs an attribute lookup of ``cls.__new__`` when +the class has a custom metaclass, so that a metaclass ``__getattribute__`` +hook observes the lookup as documented. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 90200a4379319d9..6826eaf4ee3133a 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -6328,7 +6328,36 @@ load_newobj(PickleState *state, UnpicklerObject *self, int use_kwargs) goto error; } - obj = ((PyTypeObject *)cls)->tp_new((PyTypeObject *)cls, args, kwargs); + if (Py_TYPE(cls) == &PyType_Type) { + /* Fast path: with the default metaclass, an attribute lookup of + cls.__new__ is not observable, so tp_new can be called + directly. */ + obj = ((PyTypeObject *)cls)->tp_new((PyTypeObject *)cls, args, + kwargs); + } + else { + /* Look __new__ up on the class so that a custom metaclass + __getattribute__ observes the lookup, as in the Python + implementation. */ + PyObject *func = PyObject_GetAttr(cls, &_Py_ID(__new__)); + if (func == NULL) { + goto error; + } + Py_ssize_t nargs = PyTuple_GET_SIZE(args); + PyObject *newargs = PyTuple_New(nargs + 1); + if (newargs == NULL) { + Py_DECREF(func); + goto error; + } + PyTuple_SET_ITEM(newargs, 0, Py_NewRef(cls)); + for (Py_ssize_t i = 0; i < nargs; i++) { + PyTuple_SET_ITEM(newargs, i + 1, + Py_NewRef(PyTuple_GET_ITEM(args, i))); + } + obj = PyObject_Call(func, newargs, kwargs); + Py_DECREF(newargs); + Py_DECREF(func); + } if (obj == NULL) { goto error; } From f22a1a516ec071580e895c7fff1cb681a25b648a Mon Sep 17 00:00:00 2001 From: chaerrypick01 Date: Mon, 17 Aug 2026 12:48:27 +0900 Subject: [PATCH 2/5] Track __new__ lookups via a class variable on the metaclass --- Lib/test/picklecommon.py | 6 +++--- Lib/test/pickletester.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/picklecommon.py b/Lib/test/picklecommon.py index a0513731d63ac63..58cbfb601650cbf 100644 --- a/Lib/test/picklecommon.py +++ b/Lib/test/picklecommon.py @@ -291,12 +291,12 @@ class MyIntWithNew2(MyIntWithNew): # For test_newobj_metaclass_lookup -metaclass_new_lookups = [] - class LookupLoggingMeta(type): + new_lookup_count = 0 + def __getattribute__(cls, name): if name == '__new__': - metaclass_new_lookups.append(name) + LookupLoggingMeta.new_lookup_count += 1 return super().__getattribute__(name) class NewInheriting(metaclass=LookupLoggingMeta): diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c426d0f73653eda..c4fddcfd263c643 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3608,11 +3608,11 @@ def test_newobj_metaclass_lookup(self): for proto in protocols[min_proto:]: with self.subTest(cls=cls, proto=proto): s = self.dumps(cls(42), proto) - metaclass_new_lookups.clear() + before = LookupLoggingMeta.new_lookup_count y = self.loads(s) self.assertIs(type(y), cls) self.assertEqual(y.value, 42) - self.assertIn('__new__', metaclass_new_lookups) + self.assertGreater(LookupLoggingMeta.new_lookup_count, before) def test_newobj_not_class(self): # Issue 24552 From 25924ec7b9d1dc6072893347d36cd61848e78574 Mon Sep 17 00:00:00 2001 From: Elly Chaewon Kim <83829352+chaerrypick01@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:50:54 +0900 Subject: [PATCH 3/5] Update Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst Co-authored-by: Donghee Na --- .../next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst b/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst index b5cf3c97034a995..032ff26bc58172e 100644 --- a/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst +++ b/Misc/NEWS.d/next/Library/2026-08-17-11-35-48.gh-issue-105250.wNaXOE.rst @@ -2,4 +2,4 @@ Fix a difference in behavior between the C and pure Python implementations of :mod:`pickle` when unpickling with the ``NEWOBJ`` and ``NEWOBJ_EX`` opcodes: the C implementation now performs an attribute lookup of ``cls.__new__`` when the class has a custom metaclass, so that a metaclass ``__getattribute__`` -hook observes the lookup as documented. +hook observes the lookup as documented. Patched by Chaewon Kim From 25eb25a963d7fb452a95fcfeaa682ae7c17d800d Mon Sep 17 00:00:00 2001 From: chaerrypick01 Date: Mon, 17 Aug 2026 15:04:16 +0900 Subject: [PATCH 4/5] Record the last __new__ lookup by class name instead of counting --- Lib/test/picklecommon.py | 6 ++++-- Lib/test/pickletester.py | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Lib/test/picklecommon.py b/Lib/test/picklecommon.py index 58cbfb601650cbf..2f61cf15ee224a2 100644 --- a/Lib/test/picklecommon.py +++ b/Lib/test/picklecommon.py @@ -292,11 +292,13 @@ class MyIntWithNew2(MyIntWithNew): # For test_newobj_metaclass_lookup class LookupLoggingMeta(type): - new_lookup_count = 0 + # Name of the last class whose __new__ was looked up through the + # metaclass. + last_new_lookup = None def __getattribute__(cls, name): if name == '__new__': - LookupLoggingMeta.new_lookup_count += 1 + LookupLoggingMeta.last_new_lookup = cls.__name__ return super().__getattribute__(name) class NewInheriting(metaclass=LookupLoggingMeta): diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index c4fddcfd263c643..877176b6750a9b2 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -3608,11 +3608,12 @@ def test_newobj_metaclass_lookup(self): for proto in protocols[min_proto:]: with self.subTest(cls=cls, proto=proto): s = self.dumps(cls(42), proto) - before = LookupLoggingMeta.new_lookup_count + LookupLoggingMeta.last_new_lookup = None y = self.loads(s) self.assertIs(type(y), cls) self.assertEqual(y.value, 42) - self.assertGreater(LookupLoggingMeta.new_lookup_count, before) + self.assertEqual(LookupLoggingMeta.last_new_lookup, + cls.__name__) def test_newobj_not_class(self): # Issue 24552 From b4dfe45c3b63042549eae5c873d43847e505d465 Mon Sep 17 00:00:00 2001 From: chaerrypick01 Date: Mon, 17 Aug 2026 15:34:41 +0900 Subject: [PATCH 5/5] Use _PyObject_Call_Prepend() in load_newobj() Export it from pycore_call.h, since _pickle can be built as a shared extension and cannot link against a hidden internal symbol. --- Include/internal/pycore_call.h | 3 ++- Modules/_pickle.c | 16 +++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_call.h b/Include/internal/pycore_call.h index a9db8860e91c06c..017c9576c96ad69 100644 --- a/Include/internal/pycore_call.h +++ b/Include/internal/pycore_call.h @@ -32,7 +32,8 @@ PyAPI_FUNC(PyObject*) _Py_CheckFunctionResult( PyObject *result, const char *where); -extern PyObject* _PyObject_Call_Prepend( +// Export for '_pickle' shared extension. +PyAPI_FUNC(PyObject*) _PyObject_Call_Prepend( PyThreadState *tstate, PyObject *callable, PyObject *obj, diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 6826eaf4ee3133a..566419d070de477 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -10,6 +10,7 @@ #include "Python.h" #include "pycore_bytesobject.h" // _PyBytesWriter +#include "pycore_call.h" // _PyObject_Call_Prepend() #include "pycore_ceval.h" // _Py_EnterRecursiveCall() #include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() #include "pycore_dict.h" // _PyDict_SetItem_Take2() @@ -6343,19 +6344,8 @@ load_newobj(PickleState *state, UnpicklerObject *self, int use_kwargs) if (func == NULL) { goto error; } - Py_ssize_t nargs = PyTuple_GET_SIZE(args); - PyObject *newargs = PyTuple_New(nargs + 1); - if (newargs == NULL) { - Py_DECREF(func); - goto error; - } - PyTuple_SET_ITEM(newargs, 0, Py_NewRef(cls)); - for (Py_ssize_t i = 0; i < nargs; i++) { - PyTuple_SET_ITEM(newargs, i + 1, - Py_NewRef(PyTuple_GET_ITEM(args, i))); - } - obj = PyObject_Call(func, newargs, kwargs); - Py_DECREF(newargs); + PyThreadState *tstate = _PyThreadState_GET(); + obj = _PyObject_Call_Prepend(tstate, func, cls, args, kwargs); Py_DECREF(func); } if (obj == NULL) {