classification
Title: module can set an exception in tp_clear
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: AWhetter, miss-islington, petr.viktorin, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2018-05-31 12:21 by serhiy.storchaka, last changed 2019-10-08 10:47 by petr.viktorin. This issue is now closed.

Files
File name Uploaded Description Edit
breaky.c AWhetter, 2019-10-04 20:41 Module that raises an exception in tp_clear
Pull Requests
URL Status Linked Edit
PR 16592 merged serhiy.storchaka, 2019-10-05 08:22
Messages (6)
msg318289 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-31 12:21
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 issue33622), but in any case it would be better to handle the failure locally in the module's tp_clear.
msg325075 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2018-09-11 22:06
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.
msg353971 - (view) Author: Ashley Whetter (AWhetter) * Date: 2019-10-04 20:41
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"
```
msg353980 - (view) Author: Ashley Whetter (AWhetter) * Date: 2019-10-05 00:00
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.
msg354182 - (view) Author: miss-islington (miss-islington) Date: 2019-10-08 10:46
New changeset d7c387384a27f37e4e3fa7890c859212c56b45b2 by Miss Islington (bot) (Serhiy Storchaka) in branch 'master':
bpo-33714: Output an exception raised in module's m_clear(). (GH-16592)
https://github.com/python/cpython/commit/d7c387384a27f37e4e3fa7890c859212c56b45b2
msg354184 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-10-08 10:47
Indeed, it can be closed.
Thanks Serhiy for the better error message!
History
Date User Action Args
2019-10-08 10:47:11petr.viktorinsetstatus: open -> closed
resolution: fixed
messages: + msg354184

stage: patch review -> resolved
2019-10-08 10:46:20miss-islingtonsetnosy: + miss-islington
messages: + msg354182
2019-10-05 08:22:34serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request16181
2019-10-05 00:00:27AWhettersetmessages: + msg353980
2019-10-04 20:41:49AWhettersetfiles: + breaky.c
nosy: + AWhetter
messages: + msg353971

2018-09-11 22:06:33petr.viktorinsetmessages: + msg325075
2018-05-31 14:18:40vstinnersetnosy: + petr.viktorin
2018-05-31 14:18:31vstinnersetnosy: + vstinner
2018-05-31 12:21:45serhiy.storchakacreate