Better error handling in PyImport_Cleanup()

[serhiy-storchaka]

BPO	33330
Nosy	@brettcannon, @ncoghlan, @pitrou, @vstinner, @ericsnowcurrently, @serhiy-storchaka, @miss-islington
PRs	bpo-33330: Improve error handling in PyImport_Cleanup(). #6564 [3.7] bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564) #6601 [3.6] bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564). #6604 [2.7] bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564). #6605 bpo-33330: Write exceptions occurred in PyImport_Cleanup() to stderr. #6606 bpo-33330: PyImport_Cleanup check for exc leaks #7068

^{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 = None
created_at = <Date 2018-04-22.08:56:30.611>
labels = ['interpreter-core', '3.8', 'type-bug', '3.7']
title = 'Better error handling in PyImport_Cleanup()'
updated_at = <Date 2018-05-23.10:36:34.627>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2018-05-23.10:36:34.627>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2018-04-22.08:56:30.611>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 33330
keywords = ['patch']
message_count = 9.0
messages = ['315592', '315751', '315756', '315757', '315766', '315767', '315915', '317367', '317381']
nosy_count = 7.0
nosy_names = ['brett.cannon', 'ncoghlan', 'pitrou', 'vstinner', 'eric.snow', 'serhiy.storchaka', 'miss-islington']
pr_nums = ['6564', '6601', '6604', '6605', '6606', '7068']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue33330'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[serhiy-storchaka]

The proposed PR adds missed checks for errors and PyErr_Clear() calls in PyImport_Cleanup().

It may be worth to use PyErr_WriteUnraisable() in place of PyErr_Clear() (newly added and existing) in most cases. But this can add an unexpected noise, and I don't wont to do this at this stage of 3.7. It could be done in 3.8 though, and backported to 3.7 later.

I'm not sure that PyImport_Cleanup() works properly if sys.modules is not a dict. At this stage a custom mapping will likely be broken when most of modules are destroyed.

[serhiy-storchaka]

New changeset e9d9494 by Serhiy Storchaka in branch 'master':
bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564)
e9d9494

[miss-islington]

New changeset 291c9d4 by Miss Islington (bot) in branch '3.7':
bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564)
291c9d4

[serhiy-storchaka]

PR 6606 replaces PyErr_Clear() with PyErr_WriteUnraisable(). This may add an unexpected noise at shutdown' but on other hand, this may be a helpful information in the case if something wrong happen. It doesn't produce additional output when run all tests, this means that that all is successful. I wonder in which circumstance that output will be occurred.

[serhiy-storchaka]

New changeset b16681f by Serhiy Storchaka in branch '3.6':
[3.6] bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564). (GH-6604)
b16681f

[serhiy-storchaka]

New changeset 55299ff by Serhiy Storchaka in branch '2.7':
[2.7] bpo-33330: Improve error handling in PyImport_Cleanup(). (GH-6564). (GH-6605)
55299ff

[serhiy-storchaka]

New changeset c1a6832 by Serhiy Storchaka in branch 'master':
bpo-33330: Write exceptions occurred in PyImport_Cleanup() to stderr. (GH-6606)
c1a6832

[vstinner]

I reopen the issue because I created the PR 7068 to propose further checks (in debug mode).

[serhiy-storchaka]

See also bpo-33379 about the bug in subinterpreters exposed when add yet one PyErr_WriteUnraisable() in PyImport_Cleanup() (it was not added because of this).

[iritkatriel]

Is there anything left to do here?

[vstinner]

I gave up on my further enhancements #7068 (comment), so I just close the issue.