Skip to content

/ cpython

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

bpo-1635741: Port _asyncio extension module to multiphase initialization(PEP 489) #18032

Open
wants to merge 2 commits into
base: master
from
Open

bpo-1635741: Port _asyncio extension module to multiphase initialization(PEP 489) #18032

wants to merge 2 commits into from
+47 −42

Conversation

@shihai1991
Copy link
Contributor

shihai1991 commented Jan 16, 2020 •
edited by bedevere-bot

https://bugs.python.org/issue1635741
@shihai1991
Port _asyncio extension module to multiphase initialization(PEP 489)
Loading status checks…
46759be
@shihai1991 shihai1991 requested review from 1st1 and asvetlov as code owners Jan 16, 2020
@blurb-it
📜🤖 Added by blurb_it.
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4f070d9
@asvetlov

This comment has been minimized.

Sign in to view
Copy link
Contributor

asvetlov commented Jan 16, 2020

Should multiphase support m_traverse/m_clear along with m_free?
@shihai1991

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

shihai1991 commented Jan 17, 2020 •
edited

Should multiphase support m_traverse/m_clear along with m_free?

From pep 489: If the Py_mod_create function does not return a module subclass, then m_size must be 0, and m_traverse, m_clear and m_free must all be NULL. Otherwise, SystemError is raised.

Looks I overlooked the detail.
@shihai1991

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

shihai1991 commented Jan 18, 2020

@asvetlov Hi, andrew. I checked pep 0489 again. IMHO, supporting m_traverse/m_clear should be pep 3121's content.

cc @encukou @Dormouse759
@asvetlov

This comment has been minimized.

Sign in to view
Copy link
Contributor

asvetlov commented Jan 18, 2020

My question is not about the exact PEP number but the correctness.
Currently, _asyncio has very many static C variables to keep references to other Python objects.
From my understanding, multiphase compatible module should support releasing these resources by m_free call. Since stored objects potentially can make cycles, m_traverse and m_clear are required as well.
Did I miss something?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1st1 1st1

@asvetlov asvetlov

Assignees
No one assigned
Labels
CLA signed awaiting review
Projects
None yet
Milestone
No milestone
4 participants
@shihai1991 @asvetlov @the-knights-who-say-ni @bedevere-bot
You can’t perform that action at this time.