[proxy] web.archive.org — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light

/ cpython

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
Merged

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

merged 3 commits into from Jan 27, 2020
+13 −1

Conversation

Copy link
Member

corona10 commented Jan 25, 2020 •
edited by bedevere-bot

https://bugs.python.org/issue39453
corona10 requested review from pablogsal and vstinner Jan 25, 2020
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 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 requested a review from serhiy-storchaka Jan 25, 2020

This comment has been minimized.

Sign in to view
Copy link
Member Author

corona10 commented Jan 25, 2020

@pablogsal @serhiy-storchaka @vstinner
All test are passed :)
rhettinger reviewed Jan 26, 2020
View changes
Objects/listobject.c Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Core and Builtins/2020-01-25-23-51-17.bpo-39453.xCOkYk.rst Outdated Show resolved Hide resolved
bpo-39453: Fix contains method of list to hold strong references of e…
Loading status checks…
6514398 
…lements.
corona10 force-pushed the corona10:bpo-39453 branch from ec79dda to 6514398 Jan 26, 2020
corona10 reviewed Jan 26, 2020
View changes
Copy link
Member Author

corona10 left a comment

@rhettinger I've updated the PR Please take a look :)
corona10 requested a review from rhettinger Jan 26, 2020
rhettinger approved these changes Jan 27, 2020
View changes
serhiy-storchaka reviewed Jan 27, 2020
View changes
Misc/NEWS.d/next/Core and Builtins/2020-01-25-23-51-17.bpo-39453.xCOkYk.rst Outdated
@@ -0,0 +1,2 @@
Fix contains method of list to hold strong references of elements while

This comment has been minimized.

Sign in to view
Copy link

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.

Sign in to view
Copy link

corona10 Jan 27, 2020

Author Member

Updated!
bpo-39453: Update NEW.d
Loading status checks…
5ca2fe0
corona10 requested a review from serhiy-storchaka Jan 27, 2020
pablogsal reviewed Jan 27, 2020
View changes
Lib/test/test_list.py Outdated Show resolved Hide resolved
pablogsal reviewed Jan 27, 2020
View changes
Misc/NEWS.d/next/Core and Builtins/2020-01-25-23-51-17.bpo-39453.xCOkYk.rst Outdated Show resolved Hide resolved
serhiy-storchaka approved these changes Jan 27, 2020
View changes
Copy link
Member

serhiy-storchaka left a comment

LGTM with changes proposed by @pablogsal. 👍
bpo-39453: Apply code review
Loading status checks…
d7e3e61
corona10 reviewed Jan 27, 2020
View changes
Copy link
Member Author

corona10 left a comment

@pablogsal @serhiy-storchaka
Thanks for the review :)
I 've updated them
corona10 requested a review from pablogsal Jan 27, 2020
pablogsal approved these changes Jan 27, 2020
View changes
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

This comment has been minimized.

Sign in to view
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.
🐍🍒⛏🤖

This comment has been minimized.

Sign in to view
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

This comment has been minimized.

Sign in to view
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

This comment has been minimized.

Sign in to view
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

This comment has been minimized.

Sign in to view
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
Reviewers

rhettinger

serhiy-storchaka

pablogsal

vstinner

Assignees

pablogsal

Labels
CLA signed needs backport to 3.6 needs backport to 3.7 needs backport to 3.8
Projects
None yet
Milestone
No milestone
7 participants