Repository navigation
Avoid suppressing all exceptions in PyObject_HasAttr() #75753
Copy link
Copy link
Closed
Labels
extension-modulesC modules in the Modules dirC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
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 2017-09-24.17:46:45.898>
labels = ['extension-modules', 'interpreter-core', 'type-bug']
title = 'Avoid suppressing all exceptions in PyObject_HasAttr()'
updated_at = <Date 2018-12-05.14:44:23.566>
user = 'https://github.com/serhiy-storchaka'bugs.python.org fields:
activity = <Date 2018-12-05.14:44:23.566>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Interpreter Core']
creation = <Date 2017-09-24.17:46:45.898>
creator = 'serhiy.storchaka'
dependencies = ['32787', '32788']
files = []
hgrepos = []
issue_num = 31572
keywords = ['patch']
message_count = 12.0
messages = ['302873', '304759', '304764', '304787', '304795', '306084', '306085', '306086', '306087', '310091', '316580', '331121']
nosy_count = 3.0
nosy_names = ['pitrou', 'vstinner', 'serhiy.storchaka']
pr_nums = ['3723', '3724', '3725', '3726', '3727', '3728', '3729', '3731', '4081']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31572'
versions = []Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
extension-modulesC modules in the Modules dirC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
serhiy-storchaka commentedon Sep 24, 2017
Initially hasattr() suppressed all raised exceptions. In bpo-2196 hasattr() was changed to suppress only Exception exceptions and propagate exceptions like SystemExit and KeyboardInterrupt. In bpo-9666 hasattr() was changed to suppress only AttributeError.
But C API functions, PyObject_HasAttr() and like, were not changed. PyObject_HasAttr() is documented as an equivalent of hasattr(), but there is undocumented difference. The C code that uses PyObject_HasAttr() starves from the same problem as the Python code that used old hasattr().
The only solution of this problem is getting rid of PyObject_HasAttr() if favor of PyObject_GetAttr(). In this issue I'm going to propose a set of PRs that replace PyObject_HasAttr() invocations in different components with more correct code.
serhiy-storchaka commentedon Oct 22, 2017
New changeset 04e36af by Serhiy Storchaka in branch 'master':
bpo-31572: Get rid of using _PyObject_HasAttrId() in pickle. (bpo-3729)
04e36af
pitrou commentedon Oct 22, 2017
I don't understand why. Why not fix PyObject_HasAttr() like hasattr() was?
serhiy-storchaka commentedon Oct 23, 2017
hasattr() can return True, False, or raise an exception. But PyObject_HasAttr() just returns an integer: 0 for False, not 0 for True. There is no way to return an error, and existing code doesn't expect that PyObject_HasAttr() returns an error. Leaking an exception from PyObject_HasAttr() will break existing code.
serhiy-storchaka commentedon Oct 23, 2017
New changeset 77af0a3 by Serhiy Storchaka in branch '3.6':
[3.6] bpo-31572: Get rid of using _PyObject_HasAttrId() in pickle. (GH-3729). (bpo-4081)
77af0a3
serhiy-storchaka commentedon Nov 11, 2017
New changeset d4f8480 by Serhiy Storchaka in branch 'master':
bpo-31572: Don't silence unexpected errors in the _warnings module. (bpo-3731)
d4f8480
serhiy-storchaka commentedon Nov 11, 2017
New changeset 1707e40 by Serhiy Storchaka in branch 'master':
bpo-31572: Silence only AttributeError when get the __copy__ attribute in itertools.tee(). (bpo-3724)
1707e40
serhiy-storchaka commentedon Nov 11, 2017
New changeset 60c3d35 by Serhiy Storchaka in branch 'master':
bpo-31572: Get rid of _PyObject_HasAttrId() in dict and OrderedDict. (bpo-3728)
60c3d35
serhiy-storchaka commentedon Nov 11, 2017
New changeset bba2239 by Serhiy Storchaka in branch 'master':
bpo-31572: Get rid of _PyObject_HasAttrId() in the ASDL parser. (bpo-3725)
bba2239
serhiy-storchaka commentedon Jan 16, 2018
New changeset 4d9aec0 by Serhiy Storchaka in branch 'master':
bpo-31572: Get rid of PyObject_HasAttr() and _PyObject_HasAttrId() in the _io module. (bpo-3726)
4d9aec0
vstinner commentedon May 14, 2018
It seems like this issue caused a regression in _warnings: please see bpo-33509.
serhiy-storchaka commentedon Dec 5, 2018
New changeset 398bd27 by Serhiy Storchaka in branch 'master':
bpo-32787: Better error handling in ctypes. (bpo-3727)
398bd27
vstinner commentedon Oct 9, 2023
I suppose that the next step is to design a migration path to avoid legacy API which suppress exceptions :-) It was discussed at: capi-workgroup/problems#54
Maybe we can start by soft deprecating old APIs.