GeezerVison

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

GeezerVison is showing github.com — gh-116738: Add critical section to dbm/gdbm context manager by yoney · Pull Request #140391 · python/cpython

Conversation

yoney commented Oct 20, 2025 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Add Py_{BEGIN,END}_CRITICAL_SECTION() to the context managers in the dbm and gdbm modules for the free threading build. Other methods are already covered.

This could be a problem in a rare corner case if a thread outlives the context manager's scope. I created a local test, and TSAN identifies the issue.

    def test_context_manager(self):
        def worker(db):
            db.close()

        with os_helper.temp_dir() as tmpdirname:
            with gdbm.open(f"{tmpdirname}/gdbm_filename", "c") as db:
                t1 = threading.Thread(target=worker, args=(db,))
                t1.start()

        t1.join()

WARNING: ThreadSanitizer: data race (pid=1672837)
  Read of size 8 at 0x7f9fdb330e48 by thread T1:
    #0 _gdbm_gdbm_close_impl /home/alper/workspace/tsan/./Modules/_gdbmmodule.c:413:15 (_gdbm.cpython-315td-x86_64-linux-gnu.so+0x2ce1) (BuildId: 42fff9900c3973fef01c9a778b42049ab0110b9f)
    #1 _gdbm_gdbm_close /home/alper/workspace/tsan/./Modules/clinic/_gdbmmodule.c.h:100:20 (_gdbm.cpython-315td-x86_64-linux-gnu.so+0x2ce1)
    #2 method_vectorcall_NOARGS /home/alper/workspace/tsan/Objects/descrobject.c:448:24 (python+0x5c662b) (BuildId: 89128bd333556519224b9af239856b3fcb80f8ab)
...

  Previous write of size 8 at 0x7f9fdb330e48 by main thread:
    #0 _gdbm_gdbm_close_impl /home/alper/workspace/tsan/./Modules/_gdbmmodule.c:416:18 (_gdbm.cpython-315td-x86_64-linux-gnu.so+0x4944) (BuildId: 42fff9900c3973fef01c9a778b42049ab0110b9f)
    #1 gdbm__exit__ /home/alper/workspace/tsan/./Modules/_gdbmmodule.c:693:12 (_gdbm.cpython-315td-x86_64-linux-gnu.so+0x4944)
    #2 method_vectorcall_VARARGS /home/alper/workspace/tsan/Objects/descrobject.c:325:24 (python+0x5c5ef9) (BuildId: 89128bd333556519224b9af239856b3fcb80f8ab)
...

SUMMARY: ThreadSanitizer: data race /home/alper/workspace/tsan/./Modules/_gdbmmodule.c:413:15 in _gdbm_gdbm_close_impl
==================

cc: @mpage @colesbury

Issue: Audit all built-in modules for thread safety #116738

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-116738: Add critical section to dbm/gdbm context manager

2a7d6f6

bedevere-app Bot mentioned this pull request

Oct 20, 2025

Audit all built-in modules for thread safety #116738

Open

colesbury requested review from colesbury and mpage

October 20, 2025 21:59

colesbury added the needs backport to 3.14 bugs and security fixes label

Oct 20, 2025

yoney marked this pull request as ready for review

October 20, 2025 22:00

yoney requested review from corona10, erlend-aasland and serhiy-storchaka as code owners

October 20, 2025 22:00

bedevere-app Bot added the awaiting review label

Oct 20, 2025

serhiy-storchaka approved these changes

Oct 22, 2025

View reviewed changes

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 Bot added awaiting merge and removed awaiting review labels

Oct 22, 2025

colesbury approved these changes

Oct 22, 2025

View reviewed changes

colesbury left a comment

Copy link

Copy Markdown

Contributor

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.

FYI, you can also use Argument Clinic for __enter__ and __exit__ functions now if you prefer Argument Clinic boilerplate to Py_BEGIN_CRITICAL_SECTION boilerplate.

Sorry, something went wrong.

Uh oh!

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

yoney reacted with thumbs up emoji

All reactions

👍 1 reaction

colesbury added the skip news label

Oct 22, 2025

colesbury merged commit d51be28 into python:main

Oct 22, 2025

4 checks passed

Uh oh!

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

miss-islington-app Bot commented Oct 22, 2025

Copy link

Copy Markdown

Thanks @yoney for the PR, and @colesbury 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 Bot removed the awaiting merge label

Oct 22, 2025

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Oct 22, 2025


          pythongh-116738: Add critical section to dbm/gdbm context manager (py…

556c3d5

…thongh-140391)

(cherry picked from commit d51be28)

Co-authored-by: Alper <[email protected]>

bedevere-app Bot commented Oct 22, 2025

Copy link

Copy Markdown

GH-140459 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 Bot removed the needs backport to 3.14 bugs and security fixes label

Oct 22, 2025

colesbury pushed a commit that referenced this pull request

Oct 22, 2025


          [3.14] gh-116738: Add critical section to dbm/gdbm context manager (g…

01b52ea

…h-140391) (gh-140459)

(cherry picked from commit d51be28)

Co-authored-by: Alper <[email protected]>

yoney deleted the ft_dbm branch

October 28, 2025 15:51

kumaraditya303 added the topic-free-threading label

Nov 20, 2025

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request

Dec 6, 2025


          pythongh-116738: Add critical section to dbm/gdbm context manager (py…

055bca5

…thongh-140391)

Reviewers

colesbury

colesbury approved these changes

serhiy-storchaka

serhiy-storchaka approved these changes

mpage

Awaiting requested review from mpage

corona10

Awaiting requested review from corona10

corona10 is a code owner

erlend-aasland

Awaiting requested review from erlend-aasland

erlend-aasland is a code owner

Uh oh!

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

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.

Uh oh!

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

4 participants

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.

GeezerVison

Conversation

yoney commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app Bot commented Oct 22, 2025

Uh oh!

bedevere-app Bot commented Oct 22, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yoney commented Oct 20, 2025 •

edited

Loading

Uh oh!

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