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

GH-94857: fix test_io refleak by kumaraditya303 · Pull Request #94858 · python/cpython · GitHub

/ cpython Public

GH-94857: fix test_io refleak - #94858

Merged
miss-islington merged 6 commits into
python:mainfrom
kumaraditya303:io
Jul 18, 2022
ConversationCommits (6)ChecksFiles changed
Merged

GH-94857: fix test_io refleak#94858
miss-islington merged 6 commits into
python:mainfrom
kumaraditya303:io

Conversation

kumaraditya303 commented Jul 14, 2022 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Contributor

kumaraditya303 mentioned this pull request Jul 14, 2022
Closed
kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 14, 2022

bedevere-bot commented Jul 14, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit cda2f1dfaf42821c623fce9b30b775c7c898fd1c 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 14, 2022
kumaraditya303 added the needs backport to 3.11 only security fixes label Jul 14, 2022
kumaraditya303 self-assigned this Jul 14, 2022
tiran requested changes Jul 14, 2022
View reviewed changes

tiran 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

There are more possible refleaks in error paths. It would be better to incref encodings eagerly and decref it in error paths.

bedevere-bot commented Jul 14, 2022

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

serhiy-storchaka reviewed Jul 14, 2022
View reviewed changes

serhiy-storchaka 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

There are still leaks in three return statements between lines 1260 and 1280.

serhiy-storchaka commented Jul 14, 2022

Copy link
Copy Markdown
Member

Concur with @tiran.

kumaraditya303 commented Jul 15, 2022

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

bedevere-bot commented Jul 15, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

bedevere-bot requested a review from tiran July 15, 2022 08:16
tiran reviewed Jul 15, 2022
View reviewed changes
Comment thread Modules/_io/textio.c
kumaraditya303 requested a review from tiran July 15, 2022 08:28
kumaraditya303 mentioned this pull request Jul 15, 2022
Merged
tiran approved these changes Jul 18, 2022
View reviewed changes

tiran 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

Looks good to me, thanks!

miss-islington merged commit 631160c into python:main Jul 18, 2022

miss-islington commented Jul 18, 2022

Copy link
Copy Markdown
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington commented Jul 18, 2022

Copy link
Copy Markdown
Contributor

Sorry @kumaraditya303, I had trouble checking out the 3.11 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 631160c262b40bf4ce3da6cd7bbb972ae2e9fc91 3.11

kumaraditya303 deleted the io branch July 18, 2022 13:50
kumaraditya303 added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Jul 18, 2022

miss-islington commented Jul 18, 2022

Copy link
Copy Markdown
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 18, 2022
pythonGH-94857: fix test_io refleak (pythonGH-94858)
6d2b36c 
(cherry picked from commit 631160c)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

bedevere-bot commented Jul 18, 2022

Copy link
Copy Markdown

GH-94959 is a backport of this pull request to the 3.11 branch.

bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 18, 2022
miss-islington added a commit that referenced this pull request Jul 18, 2022
GH-94857: fix test_io refleak (GH-94858)
a914fa9 
(cherry picked from commit 631160c)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka serhiy-storchaka left review comments
tiran tiran approved these changes

Assignees

miss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL