Skip to content

GH-94857: fix test_io refleak#94858

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
Merged
miss-islington merged 6 commits into from
Jul 18, 2022
Merged

GH-94857: fix test_io refleak #94858

miss-islington merged 6 commits into from
Jul 18, 2022

Conversation

@kumaraditya303
Copy link
Contributor

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

@kumaraditya303kumaraditya303 mentioned this pull request Jul 14, 2022
Closed
@kumaraditya303kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 14, 2022
@bedevere-bot
Copy link

bedevere-bot commented Jul 14, 2022

🤖 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-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 14, 2022
@kumaraditya303kumaraditya303 added the needs backport to 3.11 only security fixes label Jul 14, 2022
@kumaraditya303kumaraditya303 self-assigned this Jul 14, 2022
tiran
tiran requested changes Jul 14, 2022
View reviewed changes
Copy link
Member

@tirantiran left a comment

Choose a reason for hiding this comment

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

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

@bedevere-bot
Copy link

bedevere-bot commented Jul 14, 2022

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
serhiy-storchaka reviewed Jul 14, 2022
View reviewed changes
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

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

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Jul 14, 2022

Concur with @tiran.

@kumaraditya303
Copy link
ContributorAuthor

kumaraditya303 commented Jul 15, 2022

I have made the requested changes; please review again

@bedevere-bot
Copy link

bedevere-bot commented Jul 15, 2022

Thanks for making the requested changes!

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

@bedevere-botbedevere-bot requested a review from tiranJuly 15, 2022 08:16
tiran
tiran reviewed Jul 15, 2022
View reviewed changes
@kumaraditya303kumaraditya303 mentioned this pull request Jul 15, 2022
Merged
tiran
tiran approved these changes Jul 18, 2022
View reviewed changes
Copy link
Member

@tirantiran left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@miss-islingtonmiss-islington merged commit 631160c into python:mainJul 18, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Jul 18, 2022

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

@miss-islington
Copy link
Contributor

miss-islington commented Jul 18, 2022

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

@kumaraditya303kumaraditya303 deleted the io branch July 18, 2022 13:50
@kumaraditya303kumaraditya303 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
Copy link
Contributor

miss-islington commented Jul 18, 2022

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
@kumaraditya303@miss-islington
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
Copy link

bedevere-bot commented Jul 18, 2022

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

@bedevere-botbedevere-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
@miss-islington@kumaraditya303
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>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@tirantirantiran approved these changes

Assignees

@miss-islingtonmiss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kumaraditya303@bedevere-bot@serhiy-storchaka@miss-islington@tiran