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.

Sign up for GitHub

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

Jump to bottom

AST change introduced tons of reference leaks #82333

Closed
vstinner opened this issue Sep 13, 2019 · 4 comments
Closed

AST change introduced tons of reference leaks #82333

vstinner opened this issue Sep 13, 2019 · 4 comments
Assignees
@DinoV
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@vstinner
Copy link
Member

BPO 38152
Nosy @vstinner, @DinoV, @eduardo-elizondo
PRs
  • bpo-38152: Fix refleak in the finalizer of AST type #16127
    		•

    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 = 'https://github.com/DinoV'
closed_at = <Date 2019-09-14.16:46:31.201>
created_at = <Date 2019-09-13.09:37:08.109>
labels = ['interpreter-core', '3.9']
title = 'AST change introduced tons of reference leaks'
updated_at = <Date 2019-09-16.07:57:11.244>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-09-16.07:57:11.244>
actor = 'vstinner'
assignee = 'dino.viehland'
closed = True
closed_date = <Date 2019-09-14.16:46:31.201>
closer = 'ammar2'
components = ['Interpreter Core']
creation = <Date 2019-09-13.09:37:08.109>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 38152
keywords = ['patch']
message_count = 4.0
messages = ['352258', '352388', '352441', '352526']
nosy_count = 3.0
nosy_names = ['vstinner', 'dino.viehland', 'eelizondo']
pr_nums = ['16127']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38152'
versions = ['Python 3.9']
    @vstinner
    Copy link
    Member Author

    The following change introduced tons of reference leaks:

    commit ac46eb4
    Author: Dino Viehland <dinoviehland@fb.com>
    Date: Wed Sep 11 10:16:34 2019 -0700

    bpo-38113: Update the Python-ast.c generator to PEP-384 (gh-15957)

Summary: This mostly migrates Python-ast.c to PEP-384 and removes all statics from the whole file. This modifies the generator itself that generates the Python-ast.c. It leaves in the usage of _PyObject_LookupAttr even though it's not fully PEP-384 compatible (this could always be shimmed in by anyone who needs it).

    Full list of tests which leaked in build:
    https://buildbot.python.org/all/#/builders/1/builds/712

    test_ast
    test_asyncio
    test_builtin
    test_capi
    test_check_c_globals
    test_clinic
    test_compile
    test_dataclasses
    test_dbm
    test_dbm_dumb
    test_decimal
    test_fstring
    test_getpass
    test_idle
    test_importlib
    test_inspect
    test_pydoc
    test_source_encoding
    test_symtable
    test_sys
    test_type_comments
    test_types
    test_unittest

    vstinner@apu$ ./python -m test -j0 -R 3:3 --fromfile=leak
    (...)
    test_dbm_dumb leaked [938, 896, 908] references, sum=2742
    test_dbm leaked [144, 144, 144] references, sum=432
    test_builtin leaked [128, 128, 128] references, sum=384
    test_clinic leaked [1935, 1933, 1935] references, sum=5803
    test_getpass leaked [21, 21, 21] references, sum=63
    test_dataclasses leaked [8, 8, 8] references, sum=24
    test_idle leaked [69, 69, 69] references, sum=207
    test_inspect leaked [539, 539, 539] references, sum=1617
    test_fstring leaked [104, 104, 104] references, sum=312
    test_type_comments leaked [1337, 1335, 1337] references, sum=4009
    test_types leaked [7, 7, 7] references, sum=21
    test_compile leaked [3690, 3688, 3690] references, sum=11068
    test_sys leaked [1, 1, 1] references, sum=3
    test_pydoc leaked [1504, 1502, 1504] references, sum=4510
    test_ast leaked [357792, 357790, 357792] references, sum=1073374
    test_ast leaked [12, 11, 12] memory blocks, sum=35
    test_unittest leaked [3803, 3801, 3803] references, sum=11407
    test_decimal leaked [841, 841, 841] references, sum=2523
    test_capi leaked [2, 2, 2] references, sum=6

    I interrupted test_asyncio test: it's too slow.

    Reverting the commit ac46eb4 fix most reference leaks, except of test_capi: bpo-38150.

    @vstinner vstinner added 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Sep 13, 2019
    @DinoV DinoV self-assigned this Sep 13, 2019
    @eduardo-elizondo
    Copy link
    Mannequin

    eduardo-elizondo mannequin commented Sep 13, 2019

    I have a fix for this coming up.

    @eduardo-elizondo
    Copy link
    Mannequin

    eduardo-elizondo mannequin commented Sep 14, 2019

    The PR has been merged so the issue can be closed now

    @vstinner
    Copy link
    Member Author

    commit 0247e80
    Author: Eddie Elizondo <eelizondo@fb.com>
    Date: Sat Sep 14 09:38:17 2019 -0400

    Fix leaks in Python-ast.c (bpo-16127)

    @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
    Assignees

    @DinoV DinoV

    Labels
    3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs)
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @DinoV @ammaraskar