module can set an exception in tp_clear

[serhiy-storchaka]

BPO	33714
Nosy	@vstinner, @encukou, @serhiy-storchaka, @miss-islington, @AWhetter
PRs	bpo-33714: Output an exception raised in module's m_clear(). #16592
Files	breaky.c: Module that raises an exception in tp_clear

^{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 2019-10-08.10:47:11.099>
created_at = <Date 2018-05-31.12:21:45.371>
labels = ['interpreter-core', '3.7', '3.8', 'type-crash']
title = 'module can set an exception in tp_clear'
updated_at = <Date 2019-10-08.10:47:11.097>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2019-10-08.10:47:11.097>
actor = 'petr.viktorin'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-08.10:47:11.099>
closer = 'petr.viktorin'
components = ['Interpreter Core']
creation = <Date 2018-05-31.12:21:45.371>
creator = 'serhiy.storchaka'
dependencies = []
files = ['48642']
hgrepos = []
issue_num = 33714
keywords = ['patch']
message_count = 6.0
messages = ['318289', '325075', '353971', '353980', '354182', '354184']
nosy_count = 5.0
nosy_names = ['vstinner', 'petr.viktorin', 'serhiy.storchaka', 'miss-islington', 'AWhetter']
pr_nums = ['16592']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue33714'
versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8']

[serhiy-storchaka]

The tp_clear handler of the module object calls a custom clear function if the PyModuleDef.m_clear field is set. This function can set an exception which will be leaked to the garbage collector. An exception in tp_clear is not expected and caused a crash in the garbage collector. In the master branch it will cause just writing a traceback to stderr (see bpo-33622), but in any case it would be better to handle the failure locally in the module's tp_clear.

[encukou]

I'm not sure what tp_clear should do in this situation.
Other than propagating the exception to the GC, the most reasonable behavior seems to be to write the exception to stderr and ignore it -- but I think having the GC do that would be more robust.

IOW, I think raising an exception from tp_clear is reasonable, and if that caused (causes?) a crash, it's a bug.

[AWhetter]

I've attached a reproduction case. Here's the setup.py that I used:

from distutils.core import Extension, setup

extension = Extension("breaky", ["breaky.c"])

setup(
    name="tp_clear",
    version="0.1.0",
    ext_modules=[extension],
)

which I built and ran with:

python setup.py build_ext --inplace
python -c "import breaky, gc; a = breaky.Breaky(); a = 1"

[AWhetter]

I've just realised that this issue was specific to tp_clear on a module and not on objects. tp_clear on modules has already been fixed in https://bugs.python.org/issue33622.
I think this is closable.

[miss-islington]

New changeset d7c3873 by Miss Islington (bot) (Serhiy Storchaka) in branch 'master':
bpo-33714: Output an exception raised in module's m_clear(). (GH-16592)
d7c3873

[encukou]

Indeed, it can be closed.
Thanks Serhiy for the better error message!