FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-40077: Fix potential refleaks of _json by shihai1991 · Pull Request #19344 · python/cpython · GitHub

/ cpython Public

bpo-40077: Fix potential refleaks of _json - #19344

Merged
vstinner merged 3 commits into
python:masterfrom
shihai1991:bpo_40077_refleaks
Apr 4, 2020
Merged

bpo-40077: Fix potential refleaks of _json#19344
vstinner merged 3 commits into
python:masterfrom
shihai1991:bpo_40077_refleaks

Conversation

shihai1991 commented Apr 3, 2020
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

@corona10 self.memo of PyScannerObject should be traversed too, right?

vstinner commented Apr 3, 2020

Copy link
Copy Markdown
Member

@pablogsal: Oh, that's an intersting bug. I consider that it should be backported to 3.7 and 3.8.

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

@pablogsal: Do you think that a NEWS entry is required for such fix?

pablogsal commented Apr 3, 2020
edited
Loading

Copy link
Copy Markdown
Member

@pablogsal: Do you think that a NEWS entry is required for such fix?

Yes, as this may fix much more things that just a memory leak. If (unlikely) the memo gets into a ref cycle, some arbitrary set of objects may not be identified as a cycle and the leak will be considerable. This is a user-visible change: a potential leak that they may be experimenting should be no more, being the leak potentially unbounded.

And yes, it should be backported.

corona10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

self.memo of PyScannerObject should be traversed too, right?

Nice catch it should be LGTM

vstinner merged commit b709302 into python:master Apr 4, 2020

Copy link
Copy Markdown
Contributor

Thanks @shihai1991 for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

Copy link
Copy Markdown
Contributor

Sorry, @shihai1991 and @vstinner, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker b709302f3125622986bd458dfb2954fda5e8366d 3.8

Copy link
Copy Markdown
Contributor

Sorry @shihai1991 and @vstinner, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker b709302f3125622986bd458dfb2954fda5e8366d 3.7

Copy link
Copy Markdown
Contributor

Thanks @shihai1991 for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

Copy link
Copy Markdown
Contributor

Sorry, @shihai1991 and @vstinner, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker b709302f3125622986bd458dfb2954fda5e8366d 3.8

vstinner commented Apr 4, 2020

Copy link
Copy Markdown
Member

@shihai1991: @miss-islington failed to backport automatically the change, there is merge conflict. You can please backport the fix 3.7 and 3.8? You can use cherry_picker or git cherry-pick -x b709302f3125622986bd458dfb2954fda5e8366d. Tell me if you need help for the backport.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL