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-127081: add critical sections to `dbm` objects by duaneg · Pull Request #132749 · python/cpython

Conversation

@duaneg

@duaneg duaneg commented Apr 20, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

The dbm_* functions are not thread-safe, naturally. Add critical sections to protect their use.

  • Issue: Thread-unsafe libc functions #127081

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@duaneg
pythongh-127081: add critical sections to dbm objects
83a05d5 
The dbm_* functions are not thread-safe, naturally. Add critical sections to
protect their use.
@duaneg
duaneg requested review from corona10, erlend-aasland and serhiy-storchaka as code owners April 20, 2025 13:09
@bedevere-app bedevere-app Bot added the awaiting review label Apr 20, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 20, 2025
Thread-unsafe libc functions #127081
Open
@picnixz picnixz changed the title gh-127081: add critical sections to dbm objects gh-127081: add critical sections to dbm objects Apr 20, 2025
@ZeroIntensity ZeroIntensity added the topic-free-threading label Apr 20, 2025
ZeroIntensity
ZeroIntensity reviewed Apr 20, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity 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.

In general, it's easier to add lock_held variations of functions instead of adding inline critical sections with goto. For example:

static int
dbm_something_lock_held(PyObject *self)
{
    /* ... */
}

static int
dbm_something(PyObject *self)
{
    int result;
    Py_BEGIN_CRITICAL_SECTION(self);
    result = dbm_something_lock_held(self);
    Py_END_CRITICAL_SECTION();
    return result;
}

Would you mind switching over to that pattern here?

Sorry, something went wrong.

Uh oh!

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

duaneg commented Apr 20, 2025

Copy link
Copy Markdown
Contributor Author

In general, it's easier to add lock_held variations of functions instead of adding inline critical sections with goto.... Would you mind switching over to that pattern here?

For sure, that would be much more robust, thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@duaneg
Switch to calling a *_lock_held variant while holding the critical se…
0774870 
…ction

instead of using gotos everywhere.
@corona10 corona10 self-assigned this Apr 23, 2025
kumaraditya303
kumaraditya303 approved these changes May 8, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels May 8, 2025
@kumaraditya303
Merge branch 'main' into pythongh-127081-dbm
a1d76d6
@corona10

corona10 commented May 14, 2025

Copy link
Copy Markdown
Member

Sorry for the delay, I will review during the PyCon sprint.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ZeroIntensity
ZeroIntensity approved these changes May 14, 2025
View reviewed changes

@ZeroIntensity ZeroIntensity 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 as well

Sorry, something went wrong.

Uh oh!

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

corona10 commented May 14, 2025

Copy link
Copy Markdown
Member
스크린샷 2025-05-14 오후 1 47 57

Would you like to update clinic again?
All reactions

Sorry, something went wrong.

Uh oh!

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

corona10 commented May 14, 2025

Copy link
Copy Markdown
Member

Oh it's not relatede to dbms.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
corona10
corona10 approved these changes May 14, 2025
View reviewed changes
@corona10
corona10 merged commit ffaeb3d into python:main May 14, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label May 14, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@duaneg @Pranjal095
pythongh-127081: add critical sections to dbm objects (pythongh-132749
f8abf21 
)
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@duaneg @taegyunkim
pythongh-127081: add critical sections to dbm objects (pythongh-132749
e4523b1 
)
@yoney yoney mentioned this pull request Sep 8, 2025
gh-116738: test dbm.gnu module on FT Python build #138467
Merged
@kumaraditya303 kumaraditya303 added the needs backport to 3.14 bugs and security fixes label Oct 12, 2025
@miss-islington-app

miss-islington-app Bot commented Oct 12, 2025

Copy link
Copy Markdown

Thanks @duaneg for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 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.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 12, 2025
@duaneg @miss-islington
pythongh-127081: add critical sections to dbm objects (pythongh-132749
ea474a0 
)

(cherry picked from commit ffaeb3d)

Co-authored-by: Duane Griffin <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 12, 2025

Copy link
Copy Markdown

GH-139996 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 Oct 12, 2025
kumaraditya303 pushed a commit that referenced this pull request Oct 12, 2025
@miss-islington @duaneg
[3.14] gh-127081: add critical sections to dbm objects (gh-132749) (#…
07eb67b 
…139996)

gh-127081: add critical sections to `dbm` objects (gh-132749)
(cherry picked from commit ffaeb3d)

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

Reviewers

@corona10 corona10 corona10 approved these changes
@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka serhiy-storchaka is a code owner

Uh oh!

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

Assignees

@corona10 corona10

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.

4 participants

@duaneg @corona10 @ZeroIntensity @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 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.