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

test_mmap are leaked #104698

Closed
Eclips4 opened this issue May 20, 2023 · 8 comments
Closed

test_mmap are leaked #104698

Eclips4 opened this issue May 20, 2023 · 8 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@Eclips4
Copy link
Member

Eclips4 commented May 20, 2023 •
edited by bedevere-bot
Loading

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
@Eclips4 Eclips4 added the type-bug An unexpected behavior, bug, or error label May 20, 2023
@Eclips4
Copy link
Member Author

Eclips4 commented May 20, 2023

PR which introduce it: #103990

@AlexWaygood
Copy link
Member

PR which introduce it: #103990

Cc. @Agent-Hellboy / @JelleZijlstra / @sunmy2019

@chgnrdv
Copy link
Contributor

chgnrdv commented May 20, 2023

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

@Eclips4
Copy link
Member Author

Eclips4 commented May 20, 2023

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.

@sunmy2019
Copy link
Member

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!

Eclips4 added a commit to Eclips4/cpython that referenced this issue May 20, 2023
@Eclips4
pythongh-104698: Fix reference leak in mmapmodule.c
f973151
@bedevere-bot bedevere-bot mentioned this issue May 20, 2023
Merged
@carljm carljm mentioned this issue May 20, 2023
Merged
@JelleZijlstra
Copy link
Member

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

pablogsal pushed a commit that referenced this issue May 21, 2023
@Eclips4 @JelleZijlstra
gh-104698: Fix reference leak in mmapmodule.c (#104700)
99b6418 
Co-authored-by: Jelle Zijlstra <[email protected]>
@bedevere-bot bedevere-bot mentioned this issue May 21, 2023
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 21, 2023
@Eclips4 @JelleZijlstra @miss-islington
pythongh-104698: Fix reference leak in mmapmodule.c (pythonGH-104700)
914dbdc 
(cherry picked from commit 99b6418)

Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
JelleZijlstra added a commit that referenced this issue May 21, 2023
@miss-islington @Eclips4 @JelleZijlstra
[3.11] gh-104698: Fix reference leak in mmapmodule.c (GH-104700) (#10…
b2e0201 
…4710)

(cherry picked from commit 99b6418)

Co-authored-by: Kirill Podoprigora <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@sunmy2019
Copy link
Member

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

Can we make it mandatory?

@Eclips4
Copy link
Member Author

Eclips4 commented May 21, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@JelleZijlstra @chgnrdv @sunmy2019 @AlexWaygood @Eclips4