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-131918: Add _ThreadLocalSqliteConnection in dbm.sqlite by meghprkh · Pull Request #131920 · python/cpython

Conversation

@meghprkh

@meghprkh meghprkh commented Mar 30, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown

I am a first-time contributor and I am not sure about the quality of the code at all, review comments will be appreciated :)

Will try to add a test later. Should this be back-ported to Python 3.13?

  • Issue: dbm.sqlite breaks multi-threaded shelve usage #131918

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@meghprkh
pythongh-131918: Add _ThreadLocalSqliteConnection in dbm.sqlite
bbbdbc3
@meghprkh
meghprkh requested review from berkerpeksag and erlend-aasland as code owners March 30, 2025 21:11
@ghost

ghost commented Mar 30, 2025 •
edited by ghost
Loading

Uh oh!

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

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot commented Mar 30, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting review label Mar 30, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Mar 30, 2025
dbm.sqlite breaks multi-threaded shelve usage #131918
Open
blurb-it Bot and others added 4 commits March 30, 2025 21:14
@blurb-it
📜🤖 Added by blurb_it.
3fd9f93
@meghprkh
Typos
9a6b1c0
@meghprkh
More typo bugs
d0eec46
@meghprkh
Actually close the connection
5ebb774
@erlend-aasland

erlend-aasland commented Mar 30, 2025

Copy link
Copy Markdown
Contributor

Should this be back-ported to Python 3.13?

IMO, yes.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brianschubert
brianschubert reviewed Mar 30, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-03-30-21-14-11.gh-issue-131918.X5ibxj.rst
@@ -0,0 +1 @@
Fixes dbm.sqlite3 for multi-threaded use-cases by using thread-local connections.

@brianschubert brianschubert Mar 30, 2025

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.

Suggested change
Fixes dbm.sqlite3 for multi-threaded use-cases by using thread-local connections.
Make :mod:`dbm.sqlite3` thread-safe by using thread-local database connections.

Sorry, something went wrong.

Uh oh!

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

meghprkh commented Mar 30, 2025

Copy link
Copy Markdown
Author

Hmm this fix doesnt really work with close, but is closing sqlite connections opened in WAL mode necesssary?

As in currently the close method closes connections from other threads, which would give the same error.
All reactions

Sorry, something went wrong.

Uh oh!

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

brianschubert commented Mar 30, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Not too familiar with sqlite3 thread-safety, but does this need to consider the case where sqlite3.threadsafety is 0, which (I think) would make forming connections from multiple threads unsafe?

There should probably be a docs update to explain exactly what thread-safety guarantees this is making (and if they depend on sqlite3.threadsafety?)
All reactions

Sorry, something went wrong.

Uh oh!

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

meghprkh commented Mar 31, 2025

Copy link
Copy Markdown
Author

Very weirdly, on my system (Fedora Linux), the sqlite3.threadsafety is set to 3 by default, but I still seem to be getting the error
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
erlend-aasland marked this pull request as draft March 31, 2025 06:01
@bedevere-app bedevere-app Bot removed the awaiting review label Mar 31, 2025
corona10
corona10 reviewed Apr 2, 2025
View reviewed changes
Comment thread Lib/dbm/sqlite3.py
wrthread = ref(thread, thread_deleted)
try:
conn = sqlite3.connect(self._uri, autocommit=True, uri=True)
self._conn[id(thread)] = conn

@corona10 corona10 Apr 2, 2025

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.

Suggested change
self._conn[id(thread)] = conn
self._conn[thread.native_id] = conn

Hmm why not using native_id?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@python-cla-bot

python-cla-bot Bot commented Apr 6, 2025

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Apr 23, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 23, 2026
@StanFromIreland

StanFromIreland commented Oct 31, 2026

Copy link
Copy Markdown
Member

Closing, its a stale draft PR and changes haven't been made.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@StanFromIreland StanFromIreland closed this Oct 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corona10 corona10 corona10 left review comments
@brianschubert brianschubert brianschubert left review comments
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag
@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 will be requested when the pull request is marked ready for review serhiy-storchaka is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

stale Stale PR or inactive for long period of time.

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.

5 participants

@meghprkh @erlend-aasland @brianschubert @StanFromIreland @corona10
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.