GeezerVison

Should multiphase support m_traverse/m_clear along with m_free?

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.

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

cc @encukou @Dormouse759

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?

I'm not available this week, so just a quick comment based on a superficial reading of this PEP:

@shihai1991, PEPs 3121 and 489 are historical documents from 2007 and 2013. Any additional information should go in the Python documentation.
Could you file a ticket to add any missing info?

@asvetlov You are right. Also, with multiphase init, multiple module objects can be created from the same extension code. That can cause problems, especially if any of the static variables are mutable or created unconditionally on module initialization.

@encukou thanks for the clarification. if (static_var != NULL) {static_var = init_var();} should be enough for _asyncio needs.
I know, module-level storage for static vars is even better but I'd like to postpone it to PEP 573 landing at least. _Py_IDENTIFIER(id) is another questionable point.

I know, module-level storage for static vars is even better but I'd like to postpone it to PEP 573 landing at least. _Py_IDENTIFIER(id) is another questionable point.

+1.

@encukou thanks for your comment, petr. PEP is fine to me now and I need spend much time to understand all the historical iterations of extension module's loading ;)

I close this PR temporarily because it's depend on much future work.
Thank you for everybody's comment.

you are welcome