Skip to content

Avoid suppressing all exceptions in PyObject_HasAttr() #75753

Closed
Closed
Avoid suppressing all exceptions in PyObject_HasAttr()#75753
Labels
extension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Description

@serhiy-storchaka
serhiy-storchaka
opened on Sep 24, 2017
BPO 31572
Nosy @pitrou, @vstinner, @serhiy-storchaka
PRs
  • bpo-32788: Better error handling in sqlite3. #3723
  • bpo-31572: Get rid of using _PyObject_HasAttrId() in itertools. #3724
  • bpo-31572: Get rid of _PyObject_HasAttrId() in the ASDL parser. #3725
  • bpo-31572: Get rid of PyObject_HasAttr() and _PyObject_HasAttrId() in io. #3726
  • bpo-32787: Better error handling in ctypes. #3727
  • bpo-31572: Get rid of _PyObject_HasAttrId() in dict and OrderedDict. #3728
  • bpo-31572: Get rid of using _PyObject_HasAttrId() in pickle. #3729
  • bpo-31572: Get rid of _PyObject_HasAttrId() and PyDict_GetItemString() in warnings. #3731
  • [3.6] bpo-31572: Get rid of using _PyObject_HasAttrId() in pickle. (GH-3729). #4081
    		•
    Dependencies
  • bpo-32787: Better error handling in ctypes
  • bpo-32788: Better error handling in sqlite3
    		•

    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 = []

    Activity

    1. serhiy-storchaka commented on Sep 24, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Sep 24, 2017
      MemberAuthor

      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.

    2. serhiy-storchaka
      added
      extension-modulesC modules in the Modules dir
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-bugAn unexpected behavior, bug, or error
      on Sep 24, 2017

    3. serhiy-storchaka commented on Oct 22, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Oct 22, 2017
      MemberAuthor

      New changeset 04e36af by Serhiy Storchaka in branch 'master':
      bpo-31572: Get rid of using _PyObject_HasAttrId() in pickle. (bpo-3729)
      04e36af

    4. pitrou commented on Oct 22, 2017

      @pitrou
      pitrou
      on Oct 22, 2017
      Member

      The only solution of this problem is getting rid of PyObject_HasAttr() if favor of PyObject_GetAttr().

      I don't understand why. Why not fix PyObject_HasAttr() like hasattr() was?

    5. serhiy-storchaka commented on Oct 23, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Oct 23, 2017
      MemberAuthor

      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.

    6. serhiy-storchaka commented on Oct 23, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Oct 23, 2017
      MemberAuthor

      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

    7. serhiy-storchaka commented on Nov 11, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 11, 2017
      MemberAuthor

      New changeset d4f8480 by Serhiy Storchaka in branch 'master':
      bpo-31572: Don't silence unexpected errors in the _warnings module. (bpo-3731)
      d4f8480

    8. serhiy-storchaka commented on Nov 11, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 11, 2017
      MemberAuthor

      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

    9. serhiy-storchaka commented on Nov 11, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 11, 2017
      MemberAuthor

      New changeset 60c3d35 by Serhiy Storchaka in branch 'master':
      bpo-31572: Get rid of _PyObject_HasAttrId() in dict and OrderedDict. (bpo-3728)
      60c3d35

    10. serhiy-storchaka commented on Nov 11, 2017

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 11, 2017
      MemberAuthor

      New changeset bba2239 by Serhiy Storchaka in branch 'master':
      bpo-31572: Get rid of _PyObject_HasAttrId() in the ASDL parser. (bpo-3725)
      bba2239

    11. serhiy-storchaka commented on Jan 16, 2018

      @serhiy-storchaka
      serhiy-storchaka
      on Jan 16, 2018
      MemberAuthor

      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

    12. vstinner commented on May 14, 2018

      @vstinner
      vstinner
      on May 14, 2018
      Member

      It seems like this issue caused a regression in _warnings: please see bpo-33509.

    13. serhiy-storchaka commented on Dec 5, 2018

      @serhiy-storchaka
      serhiy-storchaka
      on Dec 5, 2018
      MemberAuthor

      New changeset 398bd27 by Serhiy Storchaka in branch 'master':
      bpo-32787: Better error handling in ctypes. (bpo-3727)
      398bd27

    14. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    15. serhiy-storchaka
      closed this as completedon Oct 9, 2023

    16. vstinner commented on Oct 9, 2023

      @vstinner
      vstinner
      on Oct 9, 2023
      Member

      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.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      extension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions