GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-148442: eliminate race condition in list rich comparison by KowalskiThomas · Pull Request #148531 · python/cpython

Conversation

@KowalskiThomas

@KowalskiThomas KowalskiThomas commented Apr 13, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

What is this PR?

This PR is a fix for #148442, where the list's rich comparison function could return an incorrect result due a race condition in non-trivial __eq__ implementations of list elements.

It also updates test_equal_operator_modifying_operand which would previously assert two different lists were equal; it now properly asserts they are different.

  • Issue: list's rich comparison has a race condition for non-trivial __eq__ and may return an incorrect result #148442

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot mentioned this pull request Apr 13, 2026
list's rich comparison has a race condition for non-trivial __eq__ and may return an incorrect result #148442
Closed
@KowalskiThomas
KowalskiThomas force-pushed the kowalski/fix-eliminate-race-condition-in-list-rich-comparison branch from cc583a3 to 3ddd03d Compare April 30, 2026 13:24
@KowalskiThomas
KowalskiThomas marked this pull request as ready for review April 30, 2026 13:25
@bedevere-app bedevere-app Bot added the awaiting review label Apr 30, 2026
serhiy-storchaka
serhiy-storchaka reviewed May 30, 2026
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could you please add a test specific to this issue, which shows why the new behavior can be better than the old one?

For example, let list1 = [x, 1] and list2 = [0, 0], where x > 0. During comparison list1 > list2, list1[0] is replaced with 0. Currently, the result will be False, even if both old and new values of list1 are larger than list2. Add also a test with replacing list2[0].

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

KowalskiThomas reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
KowalskiThomas added 5 commits May 30, 2026 17:34
@KowalskiThomas
fix: eliminate race condition in list rich comparison
be6338e
@KowalskiThomas
refactor: hoist the comparison logic
8d42bf6
@KowalskiThomas
test: update test_equal_operator_modifying_operand
21706fc
@KowalskiThomas
news: add entry
51ddb47
@KowalskiThomas
test: add regression test
b4ebf34
@KowalskiThomas
KowalskiThomas force-pushed the kowalski/fix-eliminate-race-condition-in-list-rich-comparison branch from 3ddd03d to b4ebf34 Compare May 30, 2026 15:45
@KowalskiThomas

KowalskiThomas commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka I added two tests (and rebased on main since the branch was pretty out of date).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka

serhiy-storchaka commented May 31, 2026

Copy link
Copy Markdown
Member

I afraid this is too heavyweight. No need to use threads if we can trigger the issue with a single thread.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KowalskiThomas
test: do not use threads
9668111
@KowalskiThomas
KowalskiThomas force-pushed the kowalski/fix-eliminate-race-condition-in-list-rich-comparison branch from 41e55ba to 9668111 Compare May 31, 2026 14:18
@KowalskiThomas

KowalskiThomas commented May 31, 2026

Copy link
Copy Markdown
Contributor Author

Ah yes, that's a very good point...

I just removed those tests and added two new ones that are single-threaded and do about the same thing with some... peculiar comparison operators.

Also, currently the release note says...

Fix a race condition in ``list.__lt__``, ``list.__le__``, ``list.__gt__``,
and ``list.__ge__``. Previously, under concurrent list mutation, the items
being compared could be replaced with other objects before the final comparison,
causing the final comparison result to be incorrect.

... should I also update it to frame it better and make it clear that it's not necessarily concurrent mutation? Even though that would be the most "usual" case I believe.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KowalskiThomas
KowalskiThomas requested a review from serhiy-storchaka May 31, 2026 14:21
@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 90 days with no activity.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Aug 30, 2026
serhiy-storchaka
serhiy-storchaka approved these changes Aug 30, 2026
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Aug 30, 2026
@serhiy-storchaka
serhiy-storchaka merged commit ec8a420 into python:main Aug 30, 2026
59 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 30, 2026
clin1234 pushed a commit to clin1234/cpython that referenced this pull request Sep 12, 2026
@KowalskiThomas @clin1234
pythongh-148442: Eliminate race condition in list rich comparison (py…
d2ef024
…thonGH-148531)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@KowalskiThomas @serhiy-storchaka
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.