bpo-32710: Fix _overlapped.Overlapped memory leaks by vstinner · Pull Request #11489 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fix memory leaks in asyncio ProactorEventLoop on overlapped operation
failure.
Changes:
- Implement the tp_clear and tp_traverse slots in the
_overlapped.Overlapped type to help to break reference cycles and
identify referrers in the garbage collector. - Always clear overlapped on failure: set type to TYPE_NOT_STARTED
and release resources.
https://bugs.python.org/issue32710
I don't think that this change deserves a NEWS entry.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that there are leaks when self->type is set to TYPE_NOT_STARTED.
allocated_buffer and user_buffer are members of mutually exclusive C union type.self->type is used to specify what union member is valid now.
I suspect that there are leaks when self->type is set to TYPE_NOT_STARTED.
Oh right... My previous memory leak fix was incomplete :-( I updated this PR to always clear the overlapped on failure, but also implemen tp_clear slot.
Does it look better now?
bpo-32710: Add tp_traverse to _overlapped.Overlapped bpo-32710: Fix _overlapped.Overlapped memory leaks
Fix memory leaks in asyncio ProactorEventLoop on overlapped operation failure.
Changes:
- Implement the tp_traverse slot in the _overlapped.Overlapped type to help to break reference cycles and identify referrers in the garbage collector.
- Always clear overlapped on failure: not only set type to TYPE_NOT_STARTED, but release also resources.
I changed my mind. It's wrong to implement tp_clear: if a buffer memory is release while an overlapped is still running, the process will crash. In practice, we shouldn't get into a case where tp_clear is called on a running ("pending") overlapped operation, but I prefer to be pendantic because overlapped are very tricky and error prone.
I amended my change to be able to edit the commit message: and to no longer define tp_clear.
It is OK to not implement tp_clear. Not every object that provides tp_traverse should provide tp_clear.
Maybe make allocated_buffer and user_buffer not members of a union? Then you can traverse/clear without depending on type.
Maybe make allocated_buffer and user_buffer not members of a union? Then you can traverse/clear without depending on type.
I don't think that it would simplify much the code. An overlapped is used for a single action and then is destroyed. Its type shouldn't change, and I'm fine with the current switch() to factorize the code.
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
Fix memory leaks in asyncio ProactorEventLoop on overlapped operation failures.
Changes:
- Implement the tp_traverse slot in the _overlapped.Overlapped type to help to break reference cycles and identify referrers in the garbage collector.
- Always clear overlapped on failure: not only set type to TYPE_NOT_STARTED, but release also resources. (cherry picked from commit 5485085)
Co-authored-by: Victor Stinner vstinner@redhat.com
miss-islington added a commit that referenced this pull request
Fix memory leaks in asyncio ProactorEventLoop on overlapped operation failures.
Changes:
- Implement the tp_traverse slot in the _overlapped.Overlapped type to help to break reference cycles and identify referrers in the garbage collector.
- Always clear overlapped on failure: not only set type to TYPE_NOT_STARTED, but release also resources. (cherry picked from commit 5485085)
Co-authored-by: Victor Stinner vstinner@redhat.com