8000 Fix `__module__` attribute for shared types by da-woods · Pull Request #6845 · cython/cython · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix __module__ attribute for shared types #6845

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix __module__ attribute for shared types #6845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a1a0be8
Fix __module__ attribute for shared types
da-woods May 6, 2025
6f554a6
XNewRef
da-woods May 6, 2025
d3ff4d6
Newref
da-woods May 6, 2025
f037493
Fix limited API
da-woods May 6, 2025
6db3e7e
Semi-colons for critical section
da-woods May 6, 2025
7b984ef
Fix limited API fuse types
da-woods May 7, 2025
eef099d
Avoid symbols not in the Limited API
da-woods May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions Cython/Utility/Coroutine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1771,7 +1771,7 @@ static PyObject *__Pyx_CoroutineAwait_no_new(PyTypeObject *type, PyObject *args,
// We are applying this to all Python versions (hence the commented out version guard)
// to make the behaviour explicit.
// #if CYTHON_USE_TYPE_SPECS
static PyObject *__Pyx_CoroutineAwait_reduce_ex(__pyx_CoroutineAwaitObject *self, PyObject *arg) {
static PyObject *__Pyx_CoroutineTypes_reduce_ex(PyObject *self, PyObject *arg) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reasons I don't understand, changing the definition of __module__ magically made coroutines pickleable under protocols 1 and 2. Therefore I just explicitly banned it using the same function as CoroutineAwait uses.

8000

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're renaming this function, I think it's worth stating in the function name that this is not a pickle implementation but something that intentionally prevents pickling.

Suggested change
static PyObject *__Pyx_CoroutineTypes_reduce_ex(PyObject *self, PyObject *arg) {
static PyObject *__Pyx_Coroutine_fail_reduce_ex(PyObject *self, PyObject *arg) {

CYTHON_UNUSED_VAR(arg);

__Pyx_TypeName self_type_name = __Pyx_PyType_GetFullyQualifiedName(Py_TYPE((PyObject*)self));
Expand All @@ -1790,8 +1790,8 @@ static PyMethodDef __pyx_CoroutineAwait_methods[] = {
{"close", (PyCFunction) __Pyx_CoroutineAwait_Close_Method, METH_NOARGS, PyDoc_STR("close() -> raise GeneratorExit inside coroutine.")},
// only needed with type-specs, but included in all versions for clarity
// #if CYTHON_USE_TYPE_SPECS
{"__reduce_ex__", (PyCFunction) __Pyx_CoroutineAwait_reduce_ex, METH_O, 0},
{"__reduce__", (PyCFunction) __Pyx_CoroutineAwait_reduce_ex, METH_NOARGS, 0},
{"__reduce_ex__", (PyCFunction) __Pyx_CoroutineTypes_reduce_ex, METH_O, 0},
{"__reduce__", (PyCFunction) __Pyx_CoroutineTypes_reduce_ex, METH_NOARGS, 0},
// #endif
{0, 0, 0, 0}
};
Expand Down Expand Up @@ -1844,20 +1844,45 @@ static PyObject *__Pyx_Coroutine_await(PyObject *coroutine) {
return __Pyx__Coroutine_await(coroutine);
}

// Getting and setting __module__ is particular tricky since if we make __module__ a descriptor,
// then it gets in the way of (type).__module__. Therefore intercept it in getattro/setattr
static PyObject* __pyx_Coroutine_getattro(__pyx_CoroutineObject *self, PyObject *attr) {
if (unlikely(PyUnicode_CompareWithASCIIString(attr, "__module__") == 0)) {
PyObject *module;
__Pyx_BEGIN_CRITICAL_SECTION(self);
module = __Pyx_XNewRef(self->gi_modulename);
__Pyx_END_CRITICAL_SECTION();
return module ? module : __Pyx_NewRef(Py_None);
}
return PyObject_GenericGetAttr((PyObject*)self, attr);
}

static int __pyx_Coroutine_setattro(__pyx_CoroutineObject *self, PyObject *attr, PyObject *value) {
if (unlikely(PyUnicode_CompareWithASCIIString(attr, "__module__") == 0)) {
__Pyx_BEGIN_CRITICAL_SECTION(self);
Py_XINCREF(value);
__Pyx_Py_XDECREF_SET(self->gi_modulename, value);
__Pyx_END_CRITICAL_SECTION();
return 0;
}
return PyObject_GenericSetAttr((PyObject*)self, attr, value);
}

static PyMethodDef __pyx_Coroutine_methods[] = {
{"send", (PyCFunction) __Pyx_Coroutine_Send, METH_O,
PyDoc_STR("send(arg) -> send 'arg' into coroutine,\nreturn next iterated value or raise StopIteration.")},
{"throw", (PyCFunction) __Pyx_Coroutine_Throw, METH_VARARGS,
PyDoc_STR("throw(typ[,val[,tb]]) -> raise exception in coroutine,\nreturn next iterated value or raise StopIteration.")},
{"close", (PyCFunction) __Pyx_Coroutine_Close_Method, METH_NOARGS, PyDoc_STR("close() -> raise GeneratorExit inside coroutine.")},
{"__reduce_ex__", (PyCFunction) __Pyx_CoroutineTypes_reduce_ex, METH_O, 0},
{"__reduce__", (PyCFunction) __Pyx_CoroutineTypes_reduce_ex, METH_NOARGS, 0},
{0, 0, 0, 0}
};

static PyMemberDef __pyx_Coroutine_memberlist[] = {
{"cr_await", T_OBJECT, offsetof(__pyx_CoroutineObject, yieldfrom), READONLY,
PyDoc_STR("object being awaited, or None")},
{"cr_code", T_OBJECT, offsetof(__pyx_CoroutineObject, gi_code), READONLY, NULL},
{"__module__", T_OBJECT, offsetof(__pyx_CoroutineObject, gi_modulename), 0, 0},
{"__weaklistoffset__", T_PYSSIZET, offsetof(__pyx_CoroutineObject, gi_weakreflist), READONLY, 0},
{0, 0, 0, 0, 0}
};
Expand All @@ -1881,7 +1906,8 @@ static PyType_Slot __pyx_CoroutineType_slots[] = {
{Py_tp_methods, (void *)__pyx_Coroutine_methods},
{Py_tp_members, (void *)__pyx_Coroutine_memberlist},
{Py_tp_getset, (void *)__pyx_Coroutine_getsets},
{Py_tp_getattro, (void *) PyObject_GenericGetAttr},
{Py_tp_getattro, (void *)__pyx_Coroutine_getattro},
{Py_tp_setattro, (void *)__pyx_Coroutine_setattro},
Comment on lines -1884 to +1910
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you run a quick micro-benchmark to see if this has any visible impact on the time it takes to look up and call .send() from Python code (e.g. from within asyncio)? Especially with all the attribute lookup optimisations in Py3.1[34]? Just so that we have a number.

#if CYTHON_USE_TP_FINALIZE
{Py_tp_finalize, (void *)__Pyx_Coroutine_del},
#endif
Expand Down
49 changes: 31 additions & 18 deletions Cython/Utility/CythonFunction.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,20 +622,37 @@ static void __Pyx_CyFunction_raise_type_error(__pyx_CyFunctionObject *func, cons
#endif
}


#if CYTHON_COMPILING_IN_LIMITED_API
static PyObject *
__Pyx_CyFunction_get_module(__pyx_CyFunctionObject *op, void *context) {
CYTHON_UNUSED_VAR(context);
return PyObject_GetAttrString(op->func, "__module__");
// Getting and setting __module__ is particular tricky since if we make __module__ a descriptor,
// then it gets in the way of (type).__module__. Therefore intercept it in getattro/setattr
static PyObject* __pyx_CyFunction_getattro(__pyx_CyFunctionObject *self, PyObject *attr) {
if (unlikely(PyUnicode_CompareWithASCIIString(attr, "__module__") == 0)) {
#if CYTHON_COMPILING_IN_LIMITED_API
return PyObject_GetAttr(self->func, attr);
#else
PyObject *module;
__Pyx_BEGIN_CRITICAL_SECTION(self);
module = __Pyx_XNewRef(((PyCFunctionObject*)self)->m_module);
__Pyx_END_CRITICAL_SECTION();
return module ? module : __Pyx_NewRef(Py_None);
#endif
}
return PyObject_GenericGetAttr((PyObject*)self, attr);
}

static int
__Pyx_CyFunction_set_module(__pyx_CyFunctionObject *op, PyObject* value, void *context) {
CYTHON_UNUSED_VAR(context);
return PyObject_SetAttrString(op->func, "__module__", value);
static int __pyx_CyFunction_setattro(__pyx_CyFunctionObject *self, PyObject *attr, PyObject *value) {
if (unlikely(PyUnicode_CompareWithASCIIString(attr, "__module__") == 0)) {
#if CYTHON_COMPILING_IN_LIMITED_API
return PyObject_SetAttr(self->func, attr, value);
#else
__Pyx_BEGIN_CRITICAL_SECTION(self);
Py_XINCREF(value);
Py_XSETREF(((PyCFunctionObject*)self)->m_module, value);
__Pyx_END_CRITICAL_SECTION();
return 0;
#endif
}
return PyObject_GenericSetAttr((PyObject*)self, attr, value);
}
#endif

static PyGetSetDef __pyx_CyFunction_getsets[] = {
{"func_doc", (getter)__Pyx_CyFunction_get_doc, (setter)__Pyx_CyFunction_set_doc, 0, 0},
Expand All @@ -657,16 +674,10 @@ static PyGetSetDef __pyx_CyFunction_getsets[] = {
{"__annotations__", (getter)__Pyx_CyFunction_get_annotations, (setter)__Pyx_CyFunction_set_annotations, 0, 0},
{"_is_coroutine", (getter)__Pyx_CyFunction_get_is_coroutine, 0, 0, 0},
// {"__signature__", (getter)__Pyx_CyFunction_get_signature, 0, 0, 0},
#if CYTHON_COMPILING_IN_LIMITED_API
{"__module__", (getter)__Pyx_CyFunction_get_module, (setter)__Pyx_CyFunction_set_module, 0, 0},
#endif
{0, 0, 0, 0, 0}
};

static PyMemberDef __pyx_CyFunction_members[] = {
#if !CYTHON_COMPILING_IN_LIMITED_API
{"__module__", T_OBJECT, offsetof(PyCFunctionObject, m_module), 0, 0},
#endif
{"__dictoffset__", T_PYSSIZET, offsetof(__pyx_CyFunctionObject, func_dict), READONLY, 0},
#if CYTHON_METH_FASTCALL
#if CYTHON_BACKPORT_VECTORCALL || CYTHON_COMPILING_IN_LIMITED_API
Expand Down Expand Up @@ -1211,6 +1222,8 @@ static PyType_Slot __pyx_CyFunctionType_slots[] = {
{Py_tp_methods, (void *)__pyx_CyFunction_methods},
{Py_tp_members, (void *)__pyx_CyFunction_members},
{Py_tp_getset, (void *)__pyx_CyFunction_getsets},
{Py_tp_getattro, (void *)__pyx_CyFunction_getattro},
{Py_tp_setattro, (void *)__pyx_CyFunction_setattro},
{Py_tp_descr_get, (void *)__Pyx_PyMethod_New},
{0, 0},
};
Expand Down Expand Up @@ -1409,7 +1422,7 @@ __pyx_FusedFunction_descr_get_locked(__pyx_FusedFunctionObject *func, PyObject *
PyObject *module;
__pyx_FusedFunctionObject *meth;
#if CYTHON_COMPILING_IN_LIMITED_API
module = __Pyx_CyFunction_get_module((__pyx_CyFunctionObject *) func, NULL);
module = PyObject_GetAttrString(((__pyx_CyFunctionObject*)func)->func, "__module__");
if ((unlikely(!module))) return NULL;
#else
module = ((PyCFunctionObject *) func)->m_module;
Expand Down
14 changes: 14 additions & 0 deletions tests/run/cyfunction.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,3 +409,17 @@ def test_firstlineno_decorated_function():
l1 = do_nothing.__code__.co_firstlineno
l2 = test_firstlineno_decorated_function.__code__.co_firstlineno
return l2 - l1

def test_module():
"""
>>> test_module.__module__
'cyfunction'
>>> type(test_module).__module__.startswith("_cython")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a bit fragile locally, because for some reason I always end up with an older cached cyfunction implementation.

It works when I manually import the module alone but not when I use runtests. I'm not sure what the CI will end up doing, but I'm fairly sure this is a caching problem and will go away when the shared lib version changes.

True
>>> test_module.__module__ = "something_else"
>>> test_module.__module__
'something_else'
>>> del test_module.__module__
>>> test_module.__module__
"""
pass
Loading
0