Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION #86102

Closed
serhiy-storchaka opened this issue Oct 4, 2020 · 3 comments
Closed

Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION #86102

serhiy-storchaka opened this issue Oct 4, 2020 · 3 comments
Labels
3.10 only security fixes topic-C-API type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 41936
Nosy @gvanrossum, @vstinner, @serhiy-storchaka
PRs
  • bpo-41936. Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION #22552
  • 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 2020-10-05.09:32:22.618>
    created_at = <Date 2020-10-04.22:20:31.657>
    labels = ['expert-C-API', 'type-feature', '3.10']
    title = 'Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION'
    updated_at = <Date 2020-10-05.09:32:22.618>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2020-10-05.09:32:22.618>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-10-05.09:32:22.618>
    closer = 'serhiy.storchaka'
    components = ['C API']
    creation = <Date 2020-10-04.22:20:31.657>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41936
    keywords = ['patch']
    message_count = 3.0
    messages = ['377978', '378002', '378010']
    nosy_count = 3.0
    nosy_names = ['gvanrossum', 'vstinner', 'serhiy.storchaka']
    pr_nums = ['22552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue41936'
    versions = ['Python 3.10']

    @serhiy-storchaka
    Copy link
    Member Author

    Macros Py_ALLOW_RECURSION and Py_END_ALLOW_RECURSION together with field recursion_critical of the PyInterpreterState structure were added in 5b22213 but were never documented. It seems that the reason of adding them was to work around the fact that PyDict_GetItem() can silence any exception including recursion error. But PyDict_GetItem() no longer used in that code and the macros are also no longer used (see bpo-41909).

    GvR proposed to remove these macros. I think that recursion_critical can be removed too.

    @serhiy-storchaka serhiy-storchaka added 3.10 only security fixes topic-C-API type-feature A feature request or enhancement labels Oct 4, 2020
    @vstinner
    Copy link
    Member

    vstinner commented Oct 5, 2020

    I failed to find users of these macros outside the CPython code base, so it sounds safe to remove it, especially if it's documented in What's New in Python 3.10.

    If someone reports the removal as a regression and describes a legit use case, we can reconsider to revert the removal. Until that, I'm fine with removing them.

    --

    I ran grep on Cython and numpy code base: I cannot find "Py_ALLOW_RECURSION", "Py_END_ALLOW_RECURSION" or "recursion_critical".

    On GitHub, I only found copies of Include/ceval.h (I looked at the first 5 pages of result):
    https://github.com/search?l=C&q=Py_ALLOW_RECURSION&type=Code

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset dcc5421 by Serhiy Storchaka in branch 'master':
    bpo-41936. Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION (GH-22552)
    dcc5421

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants