diff --git a/Misc/NEWS.d/next/Library/2025-05-26-17-06-40.gh-issue-134637.9-3zRL.rst b/Misc/NEWS.d/next/Library/2025-05-26-17-06-40.gh-issue-134637.9-3zRL.rst new file mode 100644 index 00000000000..2a4d8725210 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-26-17-06-40.gh-issue-134637.9-3zRL.rst @@ -0,0 +1 @@ +Fix performance regression in calling a :mod:`ctypes` function pointer in :term:`free threading`. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 7de6bb396b0..7e8a133caa7 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -3610,6 +3610,45 @@ generic_pycdata_new(ctypes_state *st, PyCFuncPtr_Type */ +static inline void +atomic_xsetref(PyObject **field, PyObject *value) +{ +#ifdef Py_GIL_DISABLED + PyObject *old = *field; + _Py_atomic_store_ptr(field, value); + Py_XDECREF(old); +#else + Py_XSETREF(*field, value); +#endif +} +/* + This function atomically loads the reference from *field, and + tries to get a new reference to it. If the incref fails, + it acquires critical section of obj and returns a new reference to the *field. + In the general case, this avoids contention on acquiring the critical section. +*/ +static inline PyObject * +atomic_xgetref(PyObject *obj, PyObject **field) +{ +#ifdef Py_GIL_DISABLED + PyObject *value = _Py_atomic_load_ptr(field); + if (value == NULL) { + return NULL; + } + if (_Py_TryIncrefCompare(field, value)) { + return value; + } + Py_BEGIN_CRITICAL_SECTION(obj); + value = Py_XNewRef(*field); + Py_END_CRITICAL_SECTION(); + return value; +#else + return Py_XNewRef(*field); +#endif +} + + + /*[clinic input] @critical_section @setter @@ -3626,7 +3665,7 @@ _ctypes_CFuncPtr_errcheck_set_impl(PyCFuncPtrObject *self, PyObject *value) return -1; } Py_XINCREF(value); - Py_XSETREF(self->errcheck, value); + atomic_xsetref(&self->errcheck, value); return 0; } @@ -3658,12 +3697,10 @@ static int _ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value) /*[clinic end generated code: output=0be0a086abbabf18 input=683c3bef4562ccc6]*/ { - PyObject *checker, *oldchecker; + PyObject *checker; if (value == NULL) { - oldchecker = self->checker; - self->checker = NULL; - Py_CLEAR(self->restype); - Py_XDECREF(oldchecker); + atomic_xsetref(&self->restype, NULL); + atomic_xsetref(&self->checker, NULL); return 0; } ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self))); @@ -3679,11 +3716,9 @@ _ctypes_CFuncPtr_restype_set_impl(PyCFuncPtrObject *self, PyObject *value) if (PyObject_GetOptionalAttr(value, &_Py_ID(_check_retval_), &checker) < 0) { return -1; } - oldchecker = self->checker; - self->checker = checker; Py_INCREF(value); - Py_XSETREF(self->restype, value); - Py_XDECREF(oldchecker); + atomic_xsetref(&self->checker, checker); + atomic_xsetref(&self->restype, value); return 0; } @@ -3728,16 +3763,16 @@ _ctypes_CFuncPtr_argtypes_set_impl(PyCFuncPtrObject *self, PyObject *value) PyObject *converters; if (value == NULL || value == Py_None) { - Py_CLEAR(self->converters); - Py_CLEAR(self->argtypes); + atomic_xsetref(&self->argtypes, NULL); + atomic_xsetref(&self->converters, NULL); } else { ctypes_state *st = get_module_state_by_def(Py_TYPE(Py_TYPE(self))); converters = converters_from_argtypes(st, value); if (!converters) return -1; - Py_XSETREF(self->converters, converters); + atomic_xsetref(&self->converters, converters); Py_INCREF(value); - Py_XSETREF(self->argtypes, value); + atomic_xsetref(&self->argtypes, value); } return 0; } @@ -4533,16 +4568,11 @@ _build_result(PyObject *result, PyObject *callargs, } static PyObject * -PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) +PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); - PyObject *restype; - PyObject *converters; - PyObject *checker; - PyObject *argtypes; - PyObject *result; - PyObject *callargs; - PyObject *errcheck; + PyObject *result = NULL; + PyObject *callargs = NULL; + PyObject *ret = NULL; #ifdef MS_WIN32 IUnknown *piunk = NULL; #endif @@ -4560,13 +4590,24 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) } assert(info); /* Cannot be NULL for PyCFuncPtrObject instances */ - restype = self->restype ? self->restype : info->restype; - converters = self->converters ? self->converters : info->converters; - checker = self->checker ? self->checker : info->checker; - argtypes = self->argtypes ? self->argtypes : info->argtypes; -/* later, we probably want to have an errcheck field in stginfo */ - errcheck = self->errcheck /* ? self->errcheck : info->errcheck */; - + PyObject *restype = atomic_xgetref(op, &self->restype); + if (restype == NULL) { + restype = Py_XNewRef(info->restype); + } + PyObject *converters = atomic_xgetref(op, &self->converters); + if (converters == NULL) { + converters = Py_XNewRef(info->converters); + } + PyObject *checker = atomic_xgetref(op, &self->checker); + if (checker == NULL) { + checker = Py_XNewRef(info->checker); + } + PyObject *argtypes = atomic_xgetref(op, &self->argtypes); + if (argtypes == NULL) { + argtypes = Py_XNewRef(info->argtypes); + } + /* later, we probably want to have an errcheck field in stginfo */ + PyObject *errcheck = atomic_xgetref(op, &self->errcheck); pProc = *(void **)self->b_ptr; #ifdef MS_WIN32 @@ -4577,25 +4618,25 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) if (!this) { PyErr_SetString(PyExc_ValueError, "native com method call without 'this' parameter"); - return NULL; + goto finally; } if (!CDataObject_Check(st, this)) { PyErr_SetString(PyExc_TypeError, "Expected a COM this pointer as first argument"); - return NULL; + goto finally; } /* there should be more checks? No, in Python */ /* First arg is a pointer to an interface instance */ if (!this->b_ptr || *(void **)this->b_ptr == NULL) { PyErr_SetString(PyExc_ValueError, "NULL COM pointer access"); - return NULL; + goto finally; } piunk = *(IUnknown **)this->b_ptr; if (NULL == piunk->lpVtbl) { PyErr_SetString(PyExc_ValueError, "COM method call without VTable"); - return NULL; + goto finally; } pProc = ((void **)piunk->lpVtbl)[self->index - 0x1000]; } @@ -4603,8 +4644,9 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) callargs = _build_callargs(st, self, argtypes, inargs, kwds, &outmask, &inoutmask, &numretvals); - if (callargs == NULL) - return NULL; + if (callargs == NULL) { + goto finally; + } if (converters) { int required = Py_SAFE_DOWNCAST(PyTuple_GET_SIZE(converters), @@ -4623,7 +4665,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) required, required == 1 ? "" : "s", actual); - return NULL; + goto finally; } } else if (required != actual) { Py_DECREF(callargs); @@ -4632,7 +4674,7 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) required, required == 1 ? "" : "s", actual); - return NULL; + goto finally; } } @@ -4663,23 +4705,19 @@ PyCFuncPtr_call_lock_held(PyObject *op, PyObject *inargs, PyObject *kwds) if (v == NULL || v != callargs) { Py_DECREF(result); Py_DECREF(callargs); - return v; + ret = v; + goto finally; } Py_DECREF(v); } - - return _build_result(result, callargs, - outmask, inoutmask, numretvals); -} - -static PyObject * -PyCFuncPtr_call(PyObject *op, PyObject *inargs, PyObject *kwds) -{ - PyObject *result; - Py_BEGIN_CRITICAL_SECTION(op); - result = PyCFuncPtr_call_lock_held(op, inargs, kwds); - Py_END_CRITICAL_SECTION(); - return result; + ret = _build_result(result, callargs, outmask, inoutmask, numretvals); +finally: + Py_XDECREF(restype); + Py_XDECREF(converters); + Py_XDECREF(checker); + Py_XDECREF(argtypes); + Py_XDECREF(errcheck); + return ret; } static int