gh-149738: Fix segmentation fault bug in sqllite3 by sepehr-rs · Pull Request #149754 · python/cpython

sepehr-rs · 2026-05-13T03:27:04Z

>>> import sqlite3
>>> db = sqlite3.connect(":memory:")
>>> del db.row_factory
>>> db.execute("test")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    db.execute("test")
    ~~~~~~~~~~^^^^^^^^
sqlite3.OperationalError: near "test": syntax error
>>>

Issue: deleting sqlite3 row_factory causes segfault #149738

picnixz

We need tests and check what happens on the created connection after deletion (check that there is still a row factory on the newly created cursor)

sepehr-rs · 2026-05-13T14:43:51Z

Thanks for your comment and review, @picnixz!
I’ve added the tests as requested and would appreciate any feedback.

P.S. The Read the Docs build is failing, but the logs indicate it’s due to an external issue unrelated to my changes.

vstinner

I would prefer using connection_getset instead of connection_members for row_factory and text_factory to raise an exception if the developer tries to delete the attribute (set it to NULL). Add get/set on these attributes.

…actory

sepehr-rs · 2026-05-21T13:02:49Z

Hi @vstinner! Thank you for your kind review. Sorry for the delay, GitHub access has been unreliable here recently.
I’ve made the requested changes and would appreciate your feedback when you have a chance.

picnixz

Plesse correct undefined behaviors by changing the getset signatures. There should be no cast to getter/setter.

bedevere-app · 2026-05-27T08:10:12Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

picnixz · 2026-05-27T08:11:39Z

 }

+static PyObject *
+connection_get_row_factory(pysqlite_Connection *self, void *closure)


Those getsets must be using a critical section as well.

sepehr-rs · 2026-05-30T11:22:47Z

Hi @picnixz, thank you for the review.

I've updated the getter/setter signatures to match the expected PyGetSetDef prototypes, removed the casts, and updated the documentation with the requested notes.

Regarding the request to use a critical section: I searched Modules/_sqlite for existing uses of Py_BEGIN_CRITICAL_SECTION (and related APIs), but couldn't find any examples in the sqlite module itself. I'm not very familiar with the free-threading synchronization patterns used in this part of CPython.

Could you please point me to an example, or clarify which object/state should be protected here?

I have made the requested changes; please review again.

picnixz · 2026-05-30T14:55:23Z

Thx. For now let's ignore races on attributes. If we can have many other attributes that can be mutated but none are protected, we can address it later.

vstinner

LGTM. I have minor concerns about the doc changes.

vstinner · 2026-06-01T12:46:02Z


+      .. versionchanged:: next
+         Deleting the ``row_factory`` attribute is no longer allowed and raises
+         :exc:`AttributeError`.


I'm not sure that it's worth it to document this minor behavior. I'm not sure that it's useful to mention the exact exception (AttributeError).

Thanks for the feedback.
I've simplified the versionchanged note and removed the mention of the specific exception. I kept the note itself since this is a user-visible behavior change, but I'm open to removing it if you think it's not necessary.

vstinner · 2026-06-01T13:17:18Z

@picnixz: Do you want to double check the change?

.

picnixz

LGTM. Should we backport it as it's a fix of a crash with an interface change though? I asked for the .. versionchanged:: stuff, but if we backport it, it will render as "3.14" for the 3.14 branch instead of 3.14.6 I think. I'm ok for the 3.15 backport at least.

vstinner · 2026-06-02T09:05:25Z

Should we backport it as it's a fix of a crash with an interface change though?

IMO it's acceptable to backport the change. There is no reason to remove these attributes. If you remove them, sqlite does crash...

I asked for the .. versionchanged:: stuff, but if we backport it, it will render as "3.14" for the 3.14 branch instead of 3.14.6 I think. I'm ok for the 3.15 backport at least.

I expect that "next" will be rendered as "3.14.6" in the 3.14 branch.

miss-islington-app · 2026-06-02T09:07:14Z

Thanks @sepehr-rs for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

bedevere-app · 2026-06-02T09:07:30Z

GH-150768 is a backport of this pull request to the 3.15 branch.

miss-islington-app · 2026-06-02T09:07:30Z

Sorry, @sepehr-rs and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 60fdb3192b897168ec0418fb0ea6c8d2d49ea513 3.13

bedevere-app · 2026-06-02T09:07:37Z

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

bedevere-app · 2026-06-02T09:10:40Z

GH-150770 is a backport of this pull request to the 3.13 branch.

vstinner · 2026-06-02T09:12:23Z

Merged, thanks for the fix @sepehr-rs. I prefer disallowing to remove attributes rather than checking if attributes are set to NULL.

…150768) gh-149738: Fix segmentation fault bug in sqllite3 (GH-149754) Deleting the `row_factory` or `text_factory` attribute is no longer allowed. (cherry picked from commit 60fdb31) Co-authored-by: Sepehr Rasouli <sepehrrasouli06@gmail.com>

sepehr-rs · 2026-06-02T09:57:08Z

Thanks everyone for the reviews and feedback!

…150769) gh-149738: Fix segmentation fault bug in sqllite3 (GH-149754) Deleting the `row_factory` or `text_factory` attribute is no longer allowed. (cherry picked from commit 60fdb31) Co-authored-by: Sepehr Rasouli <sepehrrasouli06@gmail.com>

…150770) gh-149738: Fix segmentation fault bug in sqllite3 (#149754) Deleting the `row_factory` or `text_factory` attribute is no longer allowed. (cherry picked from commit 60fdb31) Co-authored-by: Sepehr Rasouli <sepehrrasouli06@gmail.com>

sepehr-rs added 2 commits May 13, 2026 06:55

Fix segfault bug in connection.c

388090c

Add news entry

3da9d0e

sepehr-rs requested a review from erlend-aasland as a code owner May 13, 2026 03:27

bedevere-app Bot added the awaiting review label May 13, 2026

bedevere-app Bot mentioned this pull request May 13, 2026

deleting sqlite3 row_factory causes segfault #149738

Closed

picnixz reviewed May 13, 2026

View reviewed changes

Add tests

6f4faa3

vstinner reviewed May 15, 2026

View reviewed changes

sepehr-rs added 3 commits May 21, 2026 16:02

Merge branch 'main' into fix-149738

cdee382

Update connection.c to raise an exception when trying to delete row_f…

a0d272a

…actory

Update test_delete_connection_row_factory to reflect new changes

5c64a63

Fix linting issues

4d75cf9

vstinner reviewed May 21, 2026

View reviewed changes

Comment thread Modules/_sqlite/connection.c

Comment thread Modules/_sqlite/connection.c Outdated

Comment thread Lib/test/test_sqlite3/test_factory.py

sepehr-rs added 3 commits May 21, 2026 16:49

Update connection.c

20ab789

Add a test for text_factory

b36aa69

Fix linting problems

0d6c3ae

vstinner reviewed May 21, 2026

View reviewed changes

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-05-13-06-54-41.gh-issue-149738.4BLFoH.rst Outdated

vstinner added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels May 21, 2026

sepehr-rs added 2 commits May 21, 2026 17:49

Update news file

6920132

Merge branch 'main' into fix-149738

7cce0ad

picnixz previously requested changes May 27, 2026

View reviewed changes

Comment thread Modules/_sqlite/connection.c Outdated

bedevere-app Bot added awaiting changes and removed awaiting review labels May 27, 2026

picnixz reviewed May 27, 2026

View reviewed changes

Update docs

bd81a8e

vstinner reviewed Jun 1, 2026

View reviewed changes

Comment thread Doc/library/sqlite3.rst Outdated

sepehr-rs added 2 commits June 1, 2026 16:06

Change 3.16 to next

030aa69

Merge branch 'main' into fix-149738

33cb091

vstinner approved these changes Jun 1, 2026

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting changes labels Jun 1, 2026

Update docs

5489c45

picnixz approved these changes Jun 1, 2026

View reviewed changes

Merge branch 'main' into fix-149738

86f7183

vstinner merged commit 60fdb31 into python:main Jun 2, 2026
55 checks passed

bedevere-app Bot removed the awaiting merge label Jun 2, 2026

bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jun 2, 2026

miss-islington-app Bot assigned vstinner Jun 2, 2026

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jun 2, 2026

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jun 2, 2026

Uh oh!

Conversation

sepehr-rs commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

sepehr-rs commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

sepehr-rs commented May 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bedevere-app Bot commented May 27, 2026

Uh oh!

Uh oh!

picnixz May 27, 2026

Choose a reason for hiding this comment

Uh oh!

sepehr-rs commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz commented May 30, 2026

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

sepehr-rs Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jun 1, 2026

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jun 2, 2026

Uh oh!

Uh oh!

miss-islington-app Bot commented Jun 2, 2026

Uh oh!

bedevere-app Bot commented Jun 2, 2026

Uh oh!

miss-islington-app Bot commented Jun 2, 2026

Uh oh!

bedevere-app Bot commented Jun 2, 2026

Uh oh!

bedevere-app Bot commented Jun 2, 2026

Uh oh!

vstinner commented Jun 2, 2026

Uh oh!

sepehr-rs commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sepehr-rs commented May 13, 2026 •

edited

Loading

sepehr-rs commented May 13, 2026 •

edited

Loading

sepehr-rs commented May 30, 2026 •

edited

Loading