[Python-Dev] PEP 489: module m_traverse called with NULL module state

[Petr Viktorin]

On 12/14/2017 12:00 PM, Antoine Pitrou wrote:
> On Thu, 14 Dec 2017 17:00:10 +1000
> Nick Coghlan <ncoghlan at gmail.com> wrote:
>> On 14 Dec. 2017 9:19 am, "Antoine Pitrou" <solipsis at pitrou.net> wrote:
>>
>>
>> Hello,
>>
>> After debugging a crash on AppVeyor for a submitter's PR
>> (see https://github.com/python/cpython/pull/4611 ), I came to the
>> following diagnosis: converting the "atexit" module (which is a
>> built-in C extension) to PEP 489 multiphase initialization can lead to
>> its m_traverse function (and presumably also m_clear and m_free) to be
>> called while not module state is yet registered: that is,
>> `PyModule_GetState(self)` when called from m_traverse returns NULL!
>>
>> Is that an expected or known subtlety?

Thank you for looking into this, Antoine!

>> Not that I'm aware of, so I'd be inclined to classify it as a bug in the
>> way we're handling multi-phase initialisation unless/until we determine
>> there's no way to preserve the existing invariant from the single phase
>> case.

Yes, it's a bug – at least in documentation.
 From initial investigation, the problem is that between the two phases 
of multi-phase init, module state is NULL, and Python code can run.
This is expected, so I'm thinking m_traverse for all modules using 
multi-phase init should have a check for NULL. And this should be 
documented.
Let's have Marcel run with this a bit further.


> Speaking of which, the doc is not very clear: is PEP 489 required for
> multi-interpreter support or is PyModule_GetState() sufficient?

I'm not exactly sure what you're asking; which doc are you referring to?

PEP 489 gives you good defaults, if you use it and avoid global state 
(roughly: C-level mutable static variables), then you should get 
multi-interpreter support for free in simple cases.
It's also possible to use PyModule_GetState() and other APIs directly. 
However, I'd like to avoid solving subinterpreter support separately 
(and slightly differently) in each module.


For a slightly bigger picture: as a part-time internship, Marcel is 
identifying where PEP 489 is inadequate, and solving the problems for 
the complex cases. This is part of better support for subinterpreter 
support in general. Going with a PEP 489-based solution with atexit 
would help us in that effort.
I'm assuming fixing the atexit bug from 2009 [0] can be delayed a bit as 
issues with PEP 489 are investigated & solved.
Does that sound fair?


[0] https://bugs.python.org/issue6531