Skip to content

bpo-1635741 - try to port pickle to multi-stage init - #21319

Closed
koubaa wants to merge 1 commit into
python:masterfrom
koubaa:bpo-1635741-pickle
Closed

bpo-1635741 - try to port pickle to multi-stage init#21319
koubaa wants to merge 1 commit into
python:masterfrom
koubaa:bpo-1635741-pickle

Conversation

@koubaa

@koubaakoubaa commented Jul 4, 2020

Copy link
Copy Markdown
Contributor

Right now this patch causes a segfault:

test_unpickle_module_race (test.test_pickle.PyUnpicklerTests) ... Segmentation fault

It's really a clever test. Anyways hopefully the cause is apparent to reviewers. Otherwise I can try to debug it on my own some more

https://bugs.python.org/issue1635741

@koubaa

Copy link
Copy Markdown
ContributorAuthor

Found the problem. PyState_FindModule is used heavily (via _Pickle_GetGlobalState) and this is only supported for single phase init. I could replace some of those with PyType_GetModuleState but some types use Py_TPFLAGS_BASETYPE which means it may not be corrent in one for subclasses.

@vstinner Does PEP 573 (or a subsequent PEP) offer any alternative to that?

@vstinner

Copy link
Copy Markdown
Member

@vstinner Does PEP 573 (or a subsequent PEP) offer any alternative to that?

Not yet. Until PEP 573 is completed, I suggest to leave extensions which require this feature unmodified. So I suggest to close this PR for now, for example.

cc @corona10@shihai1991@encukou

@koubaakoubaa closed this Jul 7, 2020
@shihai1991

Copy link
Copy Markdown
Member

ps: you can attention petr's thread if you have free time: https://mail.python.org/archives/list/capi-sig@python.org/thread/T3P2QNLNLBRFHWSKYSTPMVEIL2EEKFJU/

@koubaa
koubaa deleted the bpo-1635741-pickle branch August 28, 2020 00:20
@koubaa
koubaa restored the bpo-1635741-pickle branch September 7, 2020 19:18
@koubaa
koubaa deleted the bpo-1635741-pickle branch November 4, 2020 03:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@koubaa@vstinner@shihai1991@the-knights-who-say-ni@bedevere-bot