-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
[C API] Revisit usage of the PyCapsule C API with multi-phase initialization API #85964
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.
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
Comments
More and more extension modules are converted to the multi-phase initialization API (PEP-489) in bpo-1635741. The problem is that the usage of the PyCapsule C API is not adapted in these extensions, to isolate well capsule objects from other module instances. For example, the pyexpat extension uses "static struct PyExpat_CAPI capi;". It was fine when it was not possible to create more than one instance of the extension module. But with PR 22222 (currently under review), it becomes possible to have multiple extension module instances. Each module instance creates its own capsule object, but all capsule object points to the same unique "struct PyExpat_CAPI" instance. For the specific case of the pyexpat in its current implementation, reusing the same "struct PyExpat_CAPI" instance is ok-ish, since the value is the same for all module instances. But this design sounds fragile. It would be safer to allocate a "struct PyExpat_CAPI" on the heap memory in each module instance, and use a PyCapsule destructor function (3rd parameter of PyCapsule_New()). The _ctypes does that: void *space = PyMem_Calloc(2, sizeof(int));
if (space == NULL)
return NULL;
errobj = PyCapsule_New(space, CTYPES_CAPSULE_NAME_PYMEM, pymem_destructor); with: static void pymem_destructor(PyObject *ptr)
{
void *p = PyCapsule_GetPointer(ptr, CTYPES_CAPSULE_NAME_PYMEM);
if (p) {
PyMem_Free(p);
}
} The PyCapsule API is used by multiple extension modules:
-- See also the PEP-630 "Isolating Extension Modules". |
Right now, it's ok-ish. Mohamed Koubaa is working on PR 22145 to pass a module state into internal functions, and so the PyCapsule pointer must be different in each module instance. |
Also note that the capsule generally needs to hold references to the objects in exposes, and not rely on the module object to keep things alive. (Module objects with multi-phase init are not unique nor immortal.) |
Oh right, that's a great advice! |
Hai Shi, do you mind if I have a go at the socket module CAPI capsule? |
Welcome to join this bpo, you can enhance those extension modules if you like :) |
Thanks :) I'll take a look at socket first. |
PySocketModuleAPI.timeout_error now points to builtin TimeoutError exception. I'm also in the process of removing PySocketModuleAPI from _ssl.c. |
That's the only user of the API, right? In that case, I'll target _decimal instead. Hai Shi is working on _curses and _datetime, AFAIK. |
BTW, do we still want to keep the socket CAPI for external users? |
Yes, at least for a while. socket.h is not part of the public include headers. It's still possible that the CAPI is used by an external project. |
I have checked all capi instances have been allocated in the heap memory. Thanks Erlend for your contribution. |
hai shi: "I have checked all capi instances have been allocated in the heap memory. So I think this bpo can be closed ;)" I also checks and I confirm that PyCapsule_New() is no longer used with a pointer pointing to static data. Well, there is one last case, in an unit test on the API itself (in _testcapi). This corner case is acceptable ;-) Thanks all for the fixes! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: