Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-135386: Fix "unable to open database file" errors on readonly DB#135566

Merged
serhiy-storchaka merged 17 commits intopython:mainfrom
GeneralK1ng:fix-issue-135386
Aug 22, 2025
Merged

gh-135386: Fix "unable to open database file" errors on readonly DB#135566
serhiy-storchaka merged 17 commits intopython:mainfrom
GeneralK1ng:fix-issue-135386

Conversation

@GeneralK1ng
Copy link
Contributor

@GeneralK1ngGeneralK1ng commentedJun 16, 2025•
edited by bedevere-appbot
Loading

What

This PR adds the SQLite URI parameterimmutable=1 when opening a database in read-only mode inLib/dbm/sqlite3.py. This explicitly informs SQLite that the database is read-only and avoids attempts to create or write to auxiliary WAL/SHM files.

Why

Without this flag, opening a read-only SQLite database may fail with errors such as "unable to open database file" when the WAL or SHM files cannot be created due to filesystem permissions or other restrictions. This is especially common when the database file is read-only and the environment prevents creation of additional files.

Addingimmutable=1 allows SQLite to operate correctly without needing to create WAL/SHM files, thus preventing these errors.

Where

The change is made in the_Database.__init__ constructor inLib/dbm/sqlite3.py. The URI used to open the database connection is appended with&immutable=1 only if theflag is"ro" (read-only).

Related issue

#135386

This is my first contribution to CPython. I appreciate the opportunity and look forward to feedback from the community. Thank you!

jmcarcell reacted with rocket emoji
@python-cla-bot
Copy link

python-cla-botbot commentedJun 16, 2025•
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

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

Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

Please add a test case. You can refer to thedevguide for how to do that.

@GeneralK1ngGeneralK1ngforce-pushed thefix-issue-135386 branch 2 times, most recently from707f4a3 to779faf2CompareJune 17, 2025 04:53
@ZeroIntensity
Copy link
Member

No need to force-push, we squash at the end.

@GeneralK1ng
Copy link
ContributorAuthor

No need to force-push, we squash at the end.

Got it, thanks for the reminder!

@ZeroIntensity
Copy link
Member

FYI, tests are failing on Windows.

GeneralK1ng reacted with thumbs up emoji

@GeneralK1ng
Copy link
ContributorAuthor

FYI, tests are failing on Windows.

Thanks for the suggestion. I've skipped the test on Windows using@unittest.skipIf(sys.platform.startswith("win"), ...) because of platform differences in file permissions and locking behavior that caused the test to fail in CI. Should be good now!

Copy link
Member

@tomasr8tomasr8 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I've skipped the test on Windows

I think we should be testing this on all platforms if possible. Maybe we can figure out why the test is failing on Windows and adapt it?

GeneralK1ng reacted with thumbs up emoji
GeneralK1ngand others added2 commitsJune 23, 2025 21:38
Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
Co-authored-by: Tomas R. <tomas.roun8@gmail.com>
@GeneralK1ng
Copy link
ContributorAuthor

I think we should be testing this on all platforms if possible. Maybe we can figure out why the test is failing on Windows and adapt it?

Sure, that makes sense — I'd be happy to look into making the test work on Windows as well.
For now, I'm developing on macOS and don't have a Windows environment set up yet.
Once I'm back home and can test on my Windows machine, I'd be glad to revisit this.

Also, thanks for the suggestions — I've adopted the URI change and simplified the flag check as suggested. : )

tomasr8 reacted with thumbs up emoji

@GeneralK1ng
Copy link
ContributorAuthor

I think we should be testing this on all platforms if possible. Maybe we can figure out why the test is failing on Windows and adapt it?

I've updated the test so it no longer skips on Windows — instead, it now explicitly closes the database before setting read-only permissions, and restores the original permissions after the test.
This should work more reliably across platforms, including Windows.
Let me know if you'd suggest any further adjustments!

@tomasr8tomasr8 self-requested a reviewJune 23, 2025 17:45
Comment on lines 94 to 115
def test_readonly_open_without_wal_shm(self):
wal_path = self.filename + "-wal"
shm_path = self.filename + "-shm"

for suffix in wal_path, shm_path:
os_helper.unlink(suffix)

try:
self.db.close()
except Exception:
pass

os.chmod(self.filename, stat.S_IREAD)

db = dbm_sqlite3.open(self.filename, "r")
try:
self.assertEqual(db[b"key1"], b"value1")
self.assertEqual(db[b"key2"], b"value2")
finally:
db.close()

os.chmod(self.filename, stat.S_IWRITE)
Copy link
Member

Choose a reason for hiding this comment

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

IIUC correctly, this change makes it so that the WAL/SHM files are no longer created in read-only mode? In that case, we could simplify the test to simply open the database in read-only mode and check that the auxiliary files are not written. I'm thinking something like this:

classImmutable(unittest.TestCase):defsetUp(self):self.filename=os_helper.TESTFNself.db=dbm_sqlite3.open(self.filename,"r")deftearDown(self):self.db.close()deftest_readonly_open_without_wal_shm(self):wal_path=self.filename+"-wal"shm_path=self.filename+"-shm"self.assertFalse(os.path.exists(wal_path))self.assertFalse(os.path.exists(shm_path))

Or does this also affect the database file itself? i.e. does passingimmutable=1 allow opening read-only files whereas otherwise it would be impossible?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks, I’ve replaced the original test with your suggested version in a new Immutable test class. It’s much cleaner and focused.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Or does this also affect the database file itself? i.e. does passingimmutable=1 allow opening read-only files whereas otherwise it would be impossible?

Yes, exactly — withoutimmutable=1, even inmode=ro, SQLite may still attempt operations that require write access (like writing lock files or checking for journal/WAL state), and will fail if the database file is read-only or in a read-only directory.

Passingimmutable=1 tells SQLite to treat the database as entirely static and read-only, which avoids those operations and allows the file to be opened successfully even with 0444 permissions or from a read-only mount.

@toslunar
Copy link

In order to useimmutable=1, we must assert the DB will not be overwritten (by other jobs). Maybedbm.open needs a newflag beyond'r' to indicate the immutability.

https://www.sqlite.org/uri.html#recognized_query_parameters says

immutable=1
The immutable query parameter is a boolean that signals to SQLite that the underlying database file is held on read-only media and cannot be modified, even by another process with elevated privileges. SQLite always opens immutable database files read-only and it skips all file locking and change detection on immutable database files. If this query parameter (or theSQLITE_IOCAP_IMMUTABLE bit in xDeviceCharacteristics) asserts that a database file is immutable and that file changes anyhow, then SQLite might return incorrect query results and/orSQLITE_CORRUPT errors.

Copy link
Contributor

@erlend-aaslanderlend-aasland left a comment

Choose a reason for hiding this comment

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

With Serhiy's amendments, this is good to go. Thank you so much for fixing this,@GeneralK1ng!

GeneralK1ng reacted with hooray emoji
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@GeneralK1ng
Copy link
ContributorAuthor

Thanks everyone for the reviews and help polishing this! Really appreciate the guidance.

@serhiy-storchakaserhiy-storchaka merged commitc0ae92b intopython:mainAug 22, 2025
44 checks passed
@serhiy-storchakaserhiy-storchaka added the needs backport to 3.14bugs and security fixes labelAug 22, 2025
@miss-islington-app
Copy link

Thanks@GeneralK1ng for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestAug 22, 2025
…y DB (pythonGH-135566)Add immutable=1 flag for read-only SQLite access to avoid WAL/SHM errors on readonly DB.(cherry picked from commitc0ae92b)Co-authored-by: General_K1ng <generak1ng0@gmail.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

GH-138056 is a backport of this pull request to the3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelAug 22, 2025
@serhiy-storchakaserhiy-storchaka added the needs backport to 3.13bugs and security fixes labelAug 22, 2025
@miss-islington-app
Copy link

Thanks@GeneralK1ng for the PR, and@serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestAug 22, 2025
…y DB (pythonGH-135566)Add immutable=1 flag for read-only SQLite access to avoid WAL/SHM errors on readonly DB.(cherry picked from commitc0ae92b)Co-authored-by: General_K1ng <generak1ng0@gmail.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

GH-138057 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelAug 22, 2025
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 Debian root 3.x (tier-1) has failed when building commitc0ae92b.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/345/builds/12027) and take a look at the build logs.
  4. Check if the failure is related to this commit (c0ae92b) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/345/builds/12027

Failed tests:

  • test_dbm_sqlite3

Failed subtests:

  • test_readonly_file_write - test.test_dbm_sqlite3.ReadOnlyFilesystem.test_readonly_file_write

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):  File"/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_dbm_sqlite3.py", line115, intest_readonly_file_writewithself.assertRaises(dbm_sqlite3.error):~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^AssertionError:error not raised

serhiy-storchaka added a commit that referenced this pull requestAug 22, 2025
…ly DB (GH-135566) (GH-138057)Add immutable=1 flag for read-only SQLite access to avoid WAL/SHM errors on readonly DB.(cherry picked from commitc0ae92b)Co-authored-by: General_K1ng <generak1ng0@gmail.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit to miss-islington/cpython that referenced this pull requestAug 22, 2025
…y DB (pythonGH-135566)Add immutable=1 flag for read-only SQLite access to avoid WAL/SHM errors on readonly DB.(cherry picked from commitc0ae92b)Co-authored-by: General_K1ng <generak1ng0@gmail.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@serhiy-storchaka
Copy link
Member

I forgot to test this under the root user.#138058 skips tests for that case.

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 Debian root 3.13 (tier-1) has failed when building commit8f22802.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1441/builds/1394) and take a look at the build logs.
  4. Check if the failure is related to this commit (8f22802) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1441/builds/1394

Failed tests:

  • test_dbm_sqlite3

Failed subtests:

  • test_readonly_file_write - test.test_dbm_sqlite3.ReadOnlyFilesystem.test_readonly_file_write

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):  File"/root/buildarea/3.13.angelico-debian-amd64/build/Lib/test/test_dbm_sqlite3.py", line116, intest_readonly_file_writewithself.assertRaises(dbm_sqlite3.error):~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^AssertionError:error not raised

@toslunar
Copy link

Isn't it a restriction ofshelve? I couldn't find a doc that says thedbm module does not support concurrent accesses.

You're right — the restriction on concurrent access is explicitly stated inshelve, notdbm.

My use ofimmutable=1 is mainly to ensure thatshelve.open(..., 'r') can work reliably on a read-only file or in a read-only directory, since otherwise SQLite may still attempt to create WAL/SHM files or acquire locks and fail.

The patch doesn’t try to enforce concurrency restrictions at thedbm level, but just ensures compatibility withshelve's expected read-only semantics.

Looking at the PR diff, it seems my concerns haven't been addressed.

@serhiy-storchaka
Copy link
Member

Support for concurrent write/read access was not promised.

@toslunar
Copy link

It's a bit surprising to me thatdbm.sqlite3's guarantee is lower than that ofsqlite3 module.

serhiy-storchaka added a commit that referenced this pull requestOct 8, 2025
…ly DB (GH-135566) (GH-138056)Add immutable=1 flag for read-only SQLite access to avoid WAL/SHM errors on readonly DB.(cherry picked from commitc0ae92b)Co-authored-by: General_K1ng <generak1ng0@gmail.com>Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>Co-authored-by: Petr Viktorin <encukou@gmail.com>
@GeneralK1ngGeneralK1ng deleted the fix-issue-135386 branchDecember 15, 2025 17:12
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka left review comments

@furkanonderfurkanonderfurkanonder left review comments

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

@berkerpeksagberkerpeksagAwaiting requested review from berkerpeksag

@ZeroIntensityZeroIntensityAwaiting requested review from ZeroIntensity

@tomasr8tomasr8Awaiting requested review from tomasr8

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@GeneralK1ng@ZeroIntensity@toslunar@serhiy-storchaka@erlend-aasland@bedevere-bot@tomasr8@furkanonder

Comments

[8]ページ先頭
©2009-2026 Movatter.jp