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

bpo-39453: Fix contains method of list to hold strong references #18181

Merged
merged 3 commits into from Jan 27, 2020

Conversation

@corona10
Copy link
Member

corona10 commented Jan 25, 2020

@corona10 corona10 requested review from pablogsal and vstinner Jan 25, 2020
@corona10 corona10 changed the title bpo-39453: Fix contains method of list to hold strong references [WIP] bpo-39453: Fix contains method of list to hold strong references Jan 25, 2020
@corona10 corona10 changed the title [WIP] bpo-39453: Fix contains method of list to hold strong references bpo-39453: Fix contains method of list to hold strong references Jan 25, 2020
@corona10 corona10 requested a review from serhiy-storchaka Jan 25, 2020
@corona10

This comment has been minimized.

Copy link
Member Author

corona10 commented Jan 25, 2020

@pablogsal @serhiy-storchaka @vstinner
All test are passed :)

@corona10 corona10 force-pushed the corona10:bpo-39453 branch from ec79dda to 6514398 Jan 26, 2020
Copy link
Member Author

corona10 left a comment

@rhettinger I've updated the PR Please take a look :)

@corona10 corona10 requested a review from rhettinger Jan 26, 2020
@@ -0,0 +1,2 @@
Fix contains method of list to hold strong references of elements while

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Jan 27, 2020

Member

It describes the implementation, but says nothing to an average Python user. I would formulate the entry in terms of Python: "Fixed possible crash in ``list.contains` when a list is changed during comparing items."

This comment has been minimized.

Copy link
@corona10

corona10 Jan 27, 2020

Author Member

Updated!

@corona10 corona10 requested a review from serhiy-storchaka Jan 27, 2020
Lib/test/test_list.py Outdated Show resolved Hide resolved
Copy link
Member

serhiy-storchaka left a comment

LGTM with changes proposed by @pablogsal. 👍

Copy link
Member Author

corona10 left a comment

@pablogsal @serhiy-storchaka
Thanks for the review :)
I 've updated them

@corona10 corona10 requested a review from pablogsal Jan 27, 2020
@pablogsal pablogsal merged commit 4dbf2d8 into python:master Jan 27, 2020
9 checks passed
9 checks passed
Docs
Details
Windows (x86)
Details
Windows (x64)
Details
macOS
Details
Ubuntu
Details
Azure Pipelines PR #20200127.17 succeeded
Details
bedevere/issue-number Issue number 39453 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Jan 27, 2020

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

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Jan 27, 2020

Sorry, @corona10 and @pablogsal, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4dbf2d8c6789a9b7299b142033073213604b8fdc 3.8

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Jan 27, 2020

Sorry @corona10 and @pablogsal, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 4dbf2d8c6789a9b7299b142033073213604b8fdc 3.7

@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Jan 27, 2020

Sorry, @corona10 and @pablogsal, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4dbf2d8c6789a9b7299b142033073213604b8fdc 3.6

@pablogsal

This comment has been minimized.

Copy link
Member

pablogsal commented Jan 27, 2020

@corona10 Could you make the backports with cherry_picker? Ping me in them and I will do the merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.