Document that m_traverse for multi-phase initialized modules can be called with m_state=NULL

[encukou]

BPO	32374
Nosy	@ncoghlan, @pitrou, @scoder, @vstinner, @encukou, @Dormouse759, @miss-islington
PRs	bpo-31901: atexit callbacks should be run at subinterpreter shutdown #4611 bpo-32374: Document that m_traverse for multi-phase initialized modules can be called with m_state=NULL #5140 [3.7] bpo-32374: m_traverse may be called with m_state=NULL (GH-5140) #6128 [3.6] bpo-32374: m_traverse may be called with m_state=NULL (GH-5140) #6129 bpo-33629: Prevent coredump in test_importlib #7090 [3.7] bpo-33629: Prevent coredump in test_importlib (GH-7090) #7101 [3.6] bpo-33629: Prevent coredump in test_importlib (GH-7090) #7102 bpo-32374: Fix test_bad_traverse. #7145 bpo-32374: Enhance test_importlib.test_bad_traverse() #7147 [3.7] bpo-32374: Ignore Python-level exceptions in test_bad_traverse (GH-7145) #7150 [3.6] bpo-32374: Ignore Python-level exceptions in test_bad_traverse (GH-7145) #7155

^{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:

assignee = None
closed_at = <Date 2018-05-15.21:20:58.194>
created_at = <Date 2017-12-19.14:40:38.428>
labels = ['extension-modules', 'type-bug', '3.7', 'docs']
title = 'Document that m_traverse for multi-phase initialized modules can be called with m_state=NULL'
updated_at = <Date 2018-05-28.16:37:14.368>
user = 'https://github.com/encukou'

bugs.python.org fields:

activity = <Date 2018-05-28.16:37:14.368>
actor = 'miss-islington'
assignee = 'docs@python'
closed = True
closed_date = <Date 2018-05-15.21:20:58.194>
closer = 'petr.viktorin'
components = ['Documentation', 'Extension Modules']
creation = <Date 2017-12-19.14:40:38.428>
creator = 'petr.viktorin'
dependencies = []
files = []
hgrepos = []
issue_num = 32374
keywords = ['patch']
message_count = 19.0
messages = ['308647', '308648', '308650', '309052', '309709', '313987', '313988', '313989', '317506', '317615', '317616', '317621', '317625', '317660', '317684', '317834', '317840', '317844', '317885']
nosy_count = 8.0
nosy_names = ['ncoghlan', 'pitrou', 'scoder', 'vstinner', 'petr.viktorin', 'docs@python', 'Dormouse759', 'miss-islington']
pr_nums = ['4611', '5140', '6128', '6129', '7090', '7101', '7102', '7145', '7147', '7150', '7155']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32374'
versions = ['Python 3.6', 'Python 3.7']

[encukou]

After the create phase of multiphase initialization, the per-module state is NULL and the module object is reachable by the garbage collector. Between the create and exec phases, Python code is run and garbage collection can be triggered.
So, any custom m_traverse/m_clear/m_free function must be prepared to handle m_state being NULL. This is currently not well documented.

It might be useful to insert a call m_traverse after the first phase, at least in --with-pydebug mode, so the potential error gets triggered early.

[encukou]

Marcel, could you look into this?

[pitrou]

By the way, I think you forgot to answer my question on python-dev:

can you get multi-interpreter support *without* PEP-489? That is, using single-phase initialization and PyModule_GetState().

The doc currently isn't very clear about this.

[ncoghlan]

Yep, the requirement for supporting multiple interpreters is just that you either avoid relying on C global state entirely, or else correctly synchronise access to it. Multi-phase initialisation just provides a few nudges in the direction of doing that more consistently.

[Dormouse759]

I have created a PR at #5140
It contains documentation, plus, in --with-pydebug mode, it calls m_traverse to catch easy cases of not handling the m_state==NULL case early.

[ncoghlan]

New changeset c2b0b12 by Nick Coghlan (Marcel Plch) in branch 'master':
bpo-32374: m_traverse may be called with m_state=NULL (GH-5140)
c2b0b12

[miss-islington]

New changeset 136905f by Miss Islington (bot) in branch '3.7':
bpo-32374: m_traverse may be called with m_state=NULL (GH-5140)
136905f

[miss-islington]

New changeset 1da0479 by Miss Islington (bot) in branch '3.6':
bpo-32374: m_traverse may be called with m_state=NULL (GH-5140)
1da0479

[vstinner]

MultiPhaseExtensionModuleTests.test_bad_traverse() of Lib/test/test_importlib/extension/test_loader.py does create a coredump file. The script run by the test trigger a segfault. Is it deliberate? See bpo-33629 and my PR 7090.