Skip to content

gh-127081: add critical sections to dbm objects - #132749

Merged
corona10 merged 3 commits into
python:mainfrom
duaneg:gh-127081-dbm
May 14, 2025
Merged

gh-127081: add critical sections to dbm objects#132749
corona10 merged 3 commits into
python:mainfrom
duaneg:gh-127081-dbm

Conversation

@duaneg

@duanegduaneg commented Apr 20, 2025

Copy link
Copy Markdown
Contributor

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

The dbm_* functions are not thread-safe, naturally. Add critical sections to
protect their use.
@picnixzpicnixz changed the title gh-127081: add critical sections to dbm objectsgh-127081: add critical sections to dbm objectsApr 20, 2025

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

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:

staticintdbm_something_lock_held(PyObject*self)
{
/* ... */
}
staticintdbm_something(PyObject*self)
{
intresult;
Py_BEGIN_CRITICAL_SECTION(self);
result=dbm_something_lock_held(self);
Py_END_CRITICAL_SECTION();
returnresult;
}

Would you mind switching over to that pattern here?

@duaneg

Copy link
Copy Markdown
ContributorAuthor

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!

@corona10corona10 self-assigned this Apr 23, 2025
@corona10

Copy link
Copy Markdown
Member

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

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM as well

@corona10

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

Would you like to update clinic again?

@corona10

Copy link
Copy Markdown
Member

Oh it's not relatede to dbms.

@corona10
corona10 merged commit ffaeb3d into python:mainMay 14, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@kumaraditya303kumaraditya303 added the needs backport to 3.14 bugs and security fixes label Oct 12, 2025
@miss-islington-app

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!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 12, 2025
)
(cherry picked from commit ffaeb3d)
Co-authored-by: Duane Griffin <duaneg@dghda.com>
@bedevere-app

Copy link
Copy Markdown

GH-139996 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot 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
…139996)
gh-127081: add critical sections to `dbm` objects (gh-132749)
(cherry picked from commit ffaeb3d)
Co-authored-by: Duane Griffin <duaneg@dghda.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@duaneg@corona10@ZeroIntensity@kumaraditya303