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

Jump to bottom

test_capi: test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once() leaks #82331

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

test_capi: test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once() leaks #82331

vstinner opened this issue Sep 13, 2019 · 9 comments
Labels
3.8 only security fixes 3.9 only security fixes tests Tests in the Lib/test dir

Comments

@vstinner
Copy link
Member

BPO 38150
Nosy @vstinner, @matrixise, @miss-islington, @eduardo-elizondo
PRs
  • bpo-38150 Fix refleak in the finalizer of a _testcapimodule type #16115
  • [3.8] bpo-38150 Fix refleak in the finalizer of a _testcapimodule type (GH-16115) #16118
    		•

    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 2019-09-13.17:16:28.678>
created_at = <Date 2019-09-13.09:15:22.434>
labels = ['3.8', 'tests', '3.9']
title = 'test_capi: test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once() leaks'
updated_at = <Date 2019-09-13.17:16:28.678>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-09-13.17:16:28.678>
actor = 'matrixise'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-13.17:16:28.678>
closer = 'matrixise'
components = ['Tests']
creation = <Date 2019-09-13.09:15:22.434>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 38150
keywords = ['patch']
message_count = 9.0
messages = ['352249', '352343', '352346', '352347', '352350', '352354', '352364', '352371', '352374']
nosy_count = 4.0
nosy_names = ['vstinner', 'matrixise', 'miss-islington', 'eelizondo']
pr_nums = ['16115', '16118']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38150'
versions = ['Python 3.8', 'Python 3.9']
    @vstinner
    Copy link
    Member Author

    vstinner@apu$ ./python -m test test_capi -R 3:3 -m test.test_capi.CAPITest.test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once
    Run tests sequentially
    0:00:00 load avg: 0.57 [1/1] test_capi
    beginning 6 repetitions
    123456
    ......
    test_capi leaked [2, 2, 2] references, sum=6
    test_capi failed

    == Tests result: FAILURE ==

    1 test failed:
    test_capi

    Total duration: 153 ms
    Tests result: FAILURE

    Note: Can't we find a shorter test name please?

    @vstinner vstinner added 3.8 only security fixes 3.9 only security fixes tests Tests in the Lib/test dir labels Sep 13, 2019
    @vstinner
    Copy link
    Member Author

    The new test was added by:

    commit ff023ed
    Author: Eddie Elizondo <eduardo.elizondorueda@gmail.com>
    Date: Wed Sep 11 05:17:13 2019 -0400

    bpo-37879: Suppress subtype_dealloc decref when base type is a C heap type (GH-15323)

    @eduardo-elizondo
    Copy link
    Mannequin

    eduardo-elizondo mannequin commented Sep 13, 2019

    On it.

    Also, I thought that the PR build bots already ran refleak tests by default? Do you know why this it's not integrated to the PR flow?

    @vstinner
    Copy link
    Member Author

    Also, I thought that the PR build bots already ran refleak tests by default? Do you know why this it's not integrated to the PR flow?

    Checking for refleak takes between 2 and 6 hours. We would like to keep our pre-commit CI fast enough (less than 30 minutes).

    @matrixise
    Copy link
    Member

    Eddie,

    For the next time, when you change something in the C part, you could run the tests with this command:

    ./python -m test test_capi -R 3:3

    @eduardo-elizondo
    Copy link
    Mannequin

    eduardo-elizondo mannequin commented Sep 13, 2019

    Checking for refleak takes between 2 and 6 hours.

    Ouch! Makes sense then.

    We could potentially add a pre-merge job that only runs once the merge starts to get executed. Anyways, that's a conversation for another time :-)

    ---

    Thanks Stephane I'm very aware of that, I just thought that the build bots did that automatically at PR time. I rather have those drive the signal and letting my slow machine run for hours! Anyways, I'll just have to slightly change my workflow now

    @miss-islington
    Copy link
    Contributor

    New changeset a67ac2f by Miss Islington (bot) (Eddie Elizondo) in branch 'master':
    bpo-38150 Fix refleak in the finalizer of a _testcapimodule type (GH-16115)
    a67ac2f

    @matrixise
    Copy link
    Member

    New changeset 53ff2ca by Stéphane Wirtel in branch '3.8':
    [3.8] bpo-38150: Fix refleak in the finalizer of a _testcapimodule type (GH-16115) (GH-16118)
    53ff2ca

    @matrixise
    Copy link
    Member

    Thank you, I close this issue.

    @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
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes tests Tests in the Lib/test dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @matrixise @vstinner @miss-islington