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

pythongh-109534: get rid of bytearray as class member #115643

Closed
wants to merge 1 commit into from

Conversation

geraldog
Copy link

@geraldog geraldog commented Feb 19, 2024

Confirmed by Issue author @rojamit that this workaround alleviates the memory leak.

This is just a simple workaround, it needs some more investigation on the C side of Python where the leak of bytearray as class member happens and why/how to fix it?

Workaround horrendous memory leaks, does not solve the
underlying issue of why bytearray as class member like
that will cause leaks, at least in this case
Copy link

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

@bedevere-app
Copy link

bedevere-app bot commented Feb 19, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@geraldog
Copy link
Author

Tests will naturally fail as the function signature has changed, but before I put any more time into this I would like to hear from the maintainers about this kind of change.

Naturally, the origin of the bug should be eventually traced and then finally after this hypothetical fix, bytearray as class member can be reinstated. That's the precise meaning of the term workaround.

Meanwhile, real users are being affected by the issue, including at least me and @rojamit very badly. That's why we need this workaround for now until we can trace the real issue with memory allocating and never freeing outside the scope of the GC.

@gvanrossum
Copy link
Member

This version isn't going to fly, the signature of buffer_updated() is documented and cannot be changed. Not even for an important work-around, sorry.

@geraldog
Copy link
Author

Understood @gvanrossum I appreciate the sincere feedback.

I do not have the time to track this further and it is beyond my expertise anyway, but let it be noted that bytearray as class member is causing at least the major part of the memory leak, and the description of the issue itself tells us 3.9 is unaffected - that's the timeframe for the introduction of bytearray as class member and I don't think it's mere coincidence.

@geraldog geraldog closed this Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants