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-146452: Fix pickle segfault on concurrent mutation of dict and list by overlorde · Pull Request #146470 · python/cpython

Conversation

@overlorde

@overlorde overlorde commented Mar 26, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

batch_dict_exact() in _pickle.c iterates dict items using PyDict_Next() which returns borrowed references. Without a critical section, a concurrent dict mutation can invalidate the borrowed reference before Py_INCREF, causing a segfault.

The fix wraps PyDict_Next() + Py_INCREF in Py_BEGIN_CRITICAL_SECTION(obj) to prevent the dict from being mutated while converting borrowed refs to owned refs. Same approach as the existing set iteration path in the same file (line 3656).

Crashes on both 3.14t (stock install) and main (with ASan). Reproducer in the linked issue.

  • Existing test_pickle passes (1000 tests, no regressions)
  • Added test_free_threading/test_pickle.py that segfaults without the fix and passes with it
  • Issue: Segfault in _pickle.c:batch_dict_exact when pickling dict with concurrent mutation (free-threading) #146452

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 Mar 26, 2026
Segfault in _pickle.c:batch_dict_exact when pickling dict with concurrent mutation (free-threading) #146452
Closed
@bedevere-app bedevere-app Bot added the awaiting review label Mar 26, 2026
@overlorde

overlorde commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

The set has a better way of doing this, should we go that way?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed Mar 27, 2026
View reviewed changes
Comment thread Modules/_pickle.c Outdated
if (dict_size - total == 1) {
/* gh-146452: Prevent concurrent dict mutation from
invalidating the borrowed refs from PyDict_Next(). */
Py_BEGIN_CRITICAL_SECTION(obj);

@vstinner vstinner Mar 27, 2026

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.

The critical section should be on the outer loop, not just on a single call according to the PyDict_Next() documentation:

      Py_BEGIN_CRITICAL_SECTION(self->dict);
      while (PyDict_Next(self->dict, &pos, &key, &value)) {
          ...
      }
      Py_END_CRITICAL_SECTION();

Sorry, something went wrong.

Uh oh!

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

@overlorde overlorde Mar 27, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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.

@vstinner thanks, there are two ways to do this, either go with the current go-to for wrapping up with the critical section, or make a new batch_dict_impl then wrap it with criticial_section_begin or critical_section_end inside the batch_dict. Tradeoff is adding an extra function, but way cleaner than the go-to approach because of its multiple layers of condition check based returns. I'm making a modification to my PR in a while.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@overlorde
pythongh-146452: Fix pickle segfault when pickling dict with concurre…
8f55561 
…nt mutation
@overlorde
overlorde force-pushed the fix-issue-146452 branch from 04b34bc to 8f55561 Compare March 27, 2026 20:25
@vstinner

vstinner commented Mar 27, 2026

Copy link
Copy Markdown
Member

Please avoid git push --force since it makes reviews harder to follow in GitHub.
overlorde reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@overlorde
Merge branch 'main' into fix-issue-146452
c4a8ee1
@overlorde

overlorde commented Mar 27, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Admitting the mistake. should i open a new pr again or work on it? i couldn't catch the UBsan failure locally. but another run fixed it on remote.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed Apr 5, 2026
View reviewed changes
Comment thread Lib/test/test_free_threading/test_pickle.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303 kumaraditya303 added topic-free-threading needs backport to 3.14 bugs and security fixes labels Apr 5, 2026
kumaraditya303
kumaraditya303 reviewed Apr 9, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2026-03-26-09-30-00.gh-issue-146452.Y2N6qZ8J.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@overlorde overlorde changed the title gh-146452: Fix pickle segfault when pickling dict with concurrent mutation gh-146452: Fix pickle segfault on concurrent mutation of dict and list Apr 27, 2026
kumaraditya303
kumaraditya303 reviewed May 17, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2026-03-26-09-30-00.gh-issue-146452.Y2N6qZ8J.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303 added 5 commits May 17, 2026 13:01
@kumaraditya303
Apply suggestion from @kumaraditya303
3e57547
@kumaraditya303
change test to use threading_helper.start_threads
6276525
@kumaraditya303
Merge branch 'main' into fix-issue-146452
f83fe04
@kumaraditya303
fix news
62b5811
@kumaraditya303
Merge branch 'fix-issue-146452' of https://github.com/overlorde/cpython…
3d305ca 
… into fix-issue-146452
kumaraditya303
kumaraditya303 approved these changes May 17, 2026
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels May 17, 2026
@kumaraditya303
kumaraditya303 merged commit e62a611 into python:main May 17, 2026
59 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented May 17, 2026

Copy link
Copy Markdown

Thanks @overlorde for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label May 17, 2026
@bedevere-app

bedevere-app Bot commented May 17, 2026

Copy link
Copy Markdown

GH-149940 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 17, 2026
kumaraditya303 added a commit that referenced this pull request May 17, 2026
@miss-islington @overlorde @kumaraditya303
[3.14] gh-146452: Fix pickle segfault on concurrent mutation of dict …
f380f85 
…in pickle (GH-146470) (#149940)

gh-146452: Fix pickle segfault on concurrent mutation of dict in pickle (GH-146470)
(cherry picked from commit e62a611)

Co-authored-by: Farhan Saif <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
@kumaraditya303 kumaraditya303 added the needs backport to 3.15 bugs and security fixes label May 23, 2026
@miss-islington-app

miss-islington-app Bot commented May 23, 2026

Copy link
Copy Markdown

Thanks @overlorde for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented May 23, 2026

Copy link
Copy Markdown

GH-150292 is a backport of this pull request to the 3.15 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label May 23, 2026
kumaraditya303 added a commit that referenced this pull request May 23, 2026
@miss-islington @overlorde @kumaraditya303
[3.15] gh-146452: Fix pickle segfault on concurrent mutation of dict …
77cc442 
…in pickle (GH-146470) (#150292)

gh-146452: Fix pickle segfault on concurrent mutation of dict in pickle (GH-146470)
(cherry picked from commit e62a611)

Co-authored-by: Farhan Saif <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

topic-free-threading

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.

3 participants

@overlorde @vstinner @kumaraditya303
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 01402520 — 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.