Description

Eclips4

opened on May 20, 2023

Issue body actions

Tried on current main branch.

 ./python -m test -R 3:3 test_mmap
Running Debug|x64 interpreter...
0:00:00 Run tests sequentially
0:00:00 [1/1] test_mmap
beginning 6 repetitions
123456
......
test_mmap leaked [13, 13, 13] references, sum=39
test_mmap failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_mmap

Total duration: 1.3 sec
Tests result: FAILURE

OS: Windows 10 & WSL Ubuntu 20.04

Linked PRs

• gh-104698: Fix reference leak in mmapmodule.c #104700
• [3.11] gh-104698: Fix reference leak in mmapmodule.c (GH-104700) #104710

Activity

1. Eclips4

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on May 20, 2023
2. Eclips4 commented on May 20, 2023

   

   Eclips4

   on May 20, 2023

   MemberAuthor

   More actions

   PR which introduce it: #103990
3. AlexWaygood commented on May 20, 2023

   

   AlexWaygood

   on May 20, 2023

   Member

   More actions

   PR which introduce it: #103990

   Cc. @Agent-Hellboy / @JelleZijlstra / @sunmy2019
4. chgnrdv commented on May 20, 2023

   

   chgnrdv

   on May 20, 2023

   Contributor

   More actions

   It seems like Py_buffer in mmap_gfind, mmap_write_method and mmap_ass_subscript should be released if CHECK_VALID fails.
5. Eclips4 commented on May 20, 2023

   

   Eclips4

   on May 20, 2023

   MemberAuthor

   More actions

   It seems like Py_buffer in mmap_gfind, mmap_write_method and mmap_ass_subscript should be released if CHECK_VALID fails.

   Yeah, you're right. I'll soon send a PR.

   Reacted by sunmy2019, chgnrdv, Oleg Iarygin and Prince Roshan
6. sunmy2019 commented on May 20, 2023

   

   sunmy2019

   on May 20, 2023

   Member

   More actions

   It seems like Py_buffer in mmap_gfind, mmap_write_method and mmap_ass_subscript should be released if CHECK_VALID fails.

   Yeah, I forgot about it.

   I'll soon send a PR.

   Thanks!

   Reacted by chgnrdv
7. Eclips4

   added a commit that references this issue on May 20, 2023

   pythongh-104698: Fix reference leak in mmapmodule.c

   f973151
8. JelleZijlstra commented on May 20, 2023

   

   JelleZijlstra

   on May 20, 2023

   Member

   More actions

   Sorry for this, should have run the refleak buildbots before merging. Thanks @chgnrdv for the diagnosis and @Eclips4 for the PR.
9. pablogsal

   added a commit that references this issue on May 21, 2023

   gh-104698: Fix reference leak in mmapmodule.c (#104700)

   99b6418
10. miss-islington

    added a commit that references this issue on May 21, 2023

    pythongh-104698: Fix reference leak in mmapmodule.c (pythonGH-104700)

    914dbdc
11. JelleZijlstra

    closed this as completedon May 21, 2023
12. JelleZijlstra

    added a commit that references this issue on May 21, 2023

    [3.11] gh-104698: Fix reference leak in mmapmodule.c (GH-104700) (#10…

    b2e0201
13. sunmy2019 commented on May 21, 2023

    

    sunmy2019

    on May 21, 2023

    Member

    More actions

    Sorry for this, should have run the refleak buildbots before merging.

    Can we make it mandatory?
14. Eclips4 commented on May 21, 2023

    

    Eclips4

    on May 21, 2023

    MemberAuthor

    More actions

    Sorry for this, should have run the refleak buildbots before merging.

    Can we make it mandatory?

    I think, it's a good idea. Also, running refleak buildbots make sense in PR's which changes/adding C code. So, there need a check.

Issue actions