Skip to content

gh-130547: Fix race between dict_dealloc and split_keys_entry_added - #130778

Merged
corona10 merged 3 commits into
python:mainfrom
corona10:gh-130547
Mar 4, 2025
Merged

gh-130547: Fix race between dict_dealloc and split_keys_entry_added#130778
corona10 merged 3 commits into
python:mainfrom
corona10:gh-130547

Conversation

@corona10

@corona10 corona10 commented Mar 3, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@corona10
corona10 requested review from colesbury and removed request for markshannon and methane March 3, 2025 09:40
@bedevere-app bedevere-app Bot mentioned this pull request Mar 3, 2025
Closed
@corona10

corona10 commented Mar 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

Other access case looks fine because of ASSERT_DICT_LOCKED

colesbury
colesbury approved these changes Mar 3, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread Objects/dictobject.c Outdated
if (values != NULL) {
if (values->embedded == 0) {
for (i = 0, n = mp->ma_keys->dk_nentries; i < n; i++) {
for (i = 0, n = FT_ATOMIC_LOAD_SSIZE_RELAXED(mp->ma_keys->dk_nentries); i < n; i++) {

@colesbury colesbury Mar 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could also do n = values->capacity, which seems a bit cleaner to me because values->capacity never changes.

@corona10
corona10 requested a review from colesbury March 4, 2025 04:55
@corona10
corona10 requested a review from mpage March 4, 2025 04:55
@corona10 corona10 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 4, 2025
@bedevere-bot

bedevere-bot commented Mar 4, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 41387ae 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130778%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 4, 2025
@corona10

corona10 commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

!buildbot Refleaks

@bedevere-bot

bedevere-bot commented Mar 4, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 41387ae 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130778%2Fmerge

The command will test the builders whose names match following regular expression: Refleaks

The builders matched are:

  • AMD64 CentOS9 Refleaks PR
  • aarch64 Fedora Stable Refleaks PR
  • PPC64LE Fedora Rawhide Refleaks PR
  • AMD64 Windows11 Refleaks PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • AMD64 RHEL8 Refleaks PR
  • AMD64 CentOS9 NoGIL Refleaks PR
  • aarch64 CentOS9 Refleaks PR
  • PPC64LE Fedora Stable Refleaks PR
  • AMD64 Fedora Rawhide Refleaks PR
  • aarch64 RHEL8 Refleaks PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • PPC64LE CentOS9 Refleaks PR
  • aarch64 Fedora Rawhide Refleaks PR
  • PPC64LE RHEL8 Refleaks PR
  • s390x RHEL8 Refleaks PR
  • AMD64 Fedora Stable Refleaks PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • s390x RHEL9 Refleaks PR
  • AMD64 FreeBSD Refleaks PR

@corona10

corona10 commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

Failures looks flasky.

@corona10
corona10 merged commit 80e6d3e into python:main Mar 4, 2025
@miss-islington-app

miss-islington-app Bot commented Mar 4, 2025

Copy link
Copy Markdown

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 4, 2025
@corona10 @miss-islington
pythongh-130547: Fix race between dict_dealloc and split_keys_entry_a…
a778b8c 
…dded (pythongh-130778)

(cherry picked from commit 80e6d3e)

Co-authored-by: Donghee Na <donghee.na@python.org>
@bedevere-app

bedevere-app Bot commented Mar 4, 2025

Copy link
Copy Markdown

GH-130833 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Mar 4, 2025
@corona10
corona10 deleted the gh-130547 branch March 5, 2025 01:41
corona10 added a commit that referenced this pull request Mar 5, 2025
@miss-islington @corona10
[3.13] gh-130547: Fix race between dict_dealloc and split_keys_entry_…
ffef9b0 
…added (gh-130778) (gh-130833)

gh-130547: Fix race between dict_dealloc and split_keys_entry_added (gh-130778)
(cherry picked from commit 80e6d3e)

Co-authored-by: Donghee Na <donghee.na@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury Awaiting requested review from colesbury
@mpage mpage Awaiting requested review from mpage

Assignees

No one assigned

Labels

skip news topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@corona10 @bedevere-bot @colesbury @mpage