Skip to content
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

Jump to bottom

gh-117398: datetime: Support per-interpreter C-API #117399

Closed
wants to merge 7 commits into from
Closed

gh-117398: datetime: Support per-interpreter C-API #117399

wants to merge 7 commits into from

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Mar 31, 2024 •
edited by bedevere-app bot
Loading

This places C-API structure on a module state, which can be accessed through PyInterpreterState or a capsule.

Doc:

For modules using multi-phase initialization, e.g. PyModule_FromDefAndSpec(),
a separate module object is created and initialized for each interpreter.

@bedevere-app bedevere-app bot mentioned this pull request Mar 31, 2024
Closed
@neonene neonene marked this pull request as draft March 31, 2024 04:33
Eclips4
Eclips4 reviewed Mar 31, 2024
View reviewed changes
Modules/_datetimemodule.c
PyObject *capsule = PyCapsule_New(capi, PyDateTime_CAPSULE_NAME,
datetime_destructor);
set_datetime_capi(st);
PyObject *capsule = PyCapsule_New(&st->capi, PyDateTime_CAPSULE_NAME, NULL);
Copy link
Member

@Eclips4 Eclips4 Mar 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can you explain why do you pass a NULL as a capsule destructor?
Is there any reason not to have destructor for this capsule?

Copy link
Contributor Author

@neonene neonene Mar 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In this case, the capsule just views a memory allocated and freed by the moduleobject.

cpython/Objects/moduleobject.c

Lines 739 to 740 in bfc57d4

if (m->md_state != NULL)
PyMem_Free(m->md_state);

Eclips4
Eclips4 reviewed Mar 31, 2024
View reviewed changes
Modules/_datetimemodule.c Outdated Show resolved Hide resolved
@neonene
Copy link
Contributor Author

neonene commented Mar 31, 2024

No futher experiment here.

@neonene neonene closed this Mar 31, 2024
@neonene
Copy link
Contributor Author

neonene commented Apr 2, 2024

On second thought, exposing a module state to a user is dangerous.

@neonene neonene deleted the dtstate branch April 2, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eclips4 Eclips4 Eclips4 left review comments

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

@abalkin abalkin Awaiting requested review from abalkin abalkin is a code owner

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neonene @Eclips4