GeezerVison

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

GeezerVison is showing github.com — bpo-31746: Prevent segfaults with uninitialised `sqlite3.Connection` objects by erlend-aasland · Pull Request #27431 · python/cpython

Conversation

erlend-aasland commented Jul 28, 2021 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Only set sqlite3.Connection.initialized when the connection object
is initialised
Check initialised flag in close()
Check initialised flag in isolation_level
Use SQLite API when enabling autocommit mode [1]
Add tests

[1] This may happen during connection init, so we cannot use
pysqlite_connection_commit, since it requires an initialised
connection object. Previously, this was not a problem, since the
initialised flag was incorrectly set before the connection object was
initialised.

https://bugs.python.org/issue31746

Sorry, something went wrong.

Uh oh!

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

All reactions


          bpo-31746: Prevent segfaults when sqlite3.Connection is uninitialised

3c12a33

erlend-aasland requested a review from berkerpeksag as a code owner

July 28, 2021 20:55

bedevere-bot added the awaiting review label

Jul 28, 2021

the-knights-who-say-ni added the CLA signed label

Jul 28, 2021

erlend-aasland requested a review from pablogsal

July 28, 2021 20:56

erlend-aasland commented

Jul 28, 2021

View reviewed changes

Comment thread

Modules/_sqlite/connection.c

Comment on lines +373 to +378

		if (!self->initialized) {
		pysqlite_state *state = pysqlite_get_state(NULL);
		PyErr_SetString(state->ProgrammingError,
		"Base Connection.__init__ not called.");
		return NULL;
		}

erlend-aasland Jul 28, 2021

Copy link

Copy Markdown

Contributor Author

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.

We cannot use pysqlite_check_connection here, since it also checks self->db. Calling close() twice should still be a no-op.

Sorry, something went wrong.

Uh oh!

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

All reactions

erlend-aasland added needs backport to 3.10 skip news labels

Jul 28, 2021

pablogsal approved these changes

Jul 29, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels

Jul 29, 2021

erlend-aasland commented Jul 29, 2021

Copy link

Copy Markdown

Contributor Author

Backport to 3.9 as well?

pablogsal reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

pablogsal merged commit 7e311e4 into python:main

Jul 29, 2021

miss-islington commented Jul 29, 2021

Copy link

Copy Markdown

Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot removed the awaiting merge label

Jul 29, 2021

miss-islington commented Jul 29, 2021

Copy link

Copy Markdown

Contributor

Sorry, @erlend-aasland and @pablogsal, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 7e311e496b0e26b3d3c62fe9b0ed2a4677c37ee9 3.10

All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal added awaiting merge needs backport to 3.9 labels

Jul 29, 2021

miss-islington assigned pablogsal

Jul 29, 2021

miss-islington commented Jul 29, 2021

Copy link

Copy Markdown

Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

miss-islington commented Jul 29, 2021

Copy link

Copy Markdown

Contributor

Sorry @erlend-aasland and @pablogsal, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 7e311e496b0e26b3d3c62fe9b0ed2a4677c37ee9 3.9

All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal commented Jul 29, 2021

Copy link

Copy Markdown

Member

Oh, seems that the backport party crashed :(

@erlend-aasland can you do them manually?

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland deleted the sqlite-uninitialised-connection branch

July 29, 2021 19:46

erlend-aasland commented Jul 29, 2021

Copy link

Copy Markdown

Contributor Author

Oh, seems that the backport party crashed :(

@erlend-aasland can you do them manually?

Yes, I was expecting it to fail. I'll fix it right away. Should we backport to 3.9 as well?

All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal commented Jul 29, 2021

Copy link

Copy Markdown

Member

@erlend-aasland apparently we have a buildbot failure:

https://buildbot.python.org/all/#/builders/596/builds/703/

We may need to revert this meanwhile we understand what failed

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request

Jul 29, 2021


          [3.9] bpo-31746: Prevent segfaults when sqlite3.Connection is uniniti…

6babf43

…alised (pythonGH-27431).

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

erlend-aasland commented Jul 29, 2021

Copy link

Copy Markdown

Contributor Author

Yes, please revert it.

All reactions

Sorry, something went wrong.

Uh oh!

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

pablogsal added a commit that referenced this pull request

Jul 29, 2021


          Revert "bpo-31746: Prevent segfaults when sqlite3.Connection is unini…

0dffd84

…tialised (GH-27431)"

This reverts commit 7e311e4.

pablogsal mentioned this pull request

Jul 29, 2021

bpo-31746: Revert "Prevent segfaults with uninitialised sqlite3.Connection objects" #27463

Closed

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request

Jul 29, 2021


          bpo-31746: Fix broken call in pythonGH-27431

ea7b889

bedevere-bot commented Jul 29, 2021

Copy link

Copy Markdown

GH-27465 is a backport of this pull request to the 3.9 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot removed the needs backport to 3.9 label

Jul 29, 2021

pablogsal pushed a commit that referenced this pull request

Jul 29, 2021


          bpo-31746: Fix broken call in GH-27431 (GH-27464)

2d34664

pablogsal pushed a commit that referenced this pull request

Jul 29, 2021


          [3.9] bpo-31746: Prevent segfaults when sqlite3.Connection is uniniti…

0bc1765

…alised (GH-27431) (GH-27465)

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

pablogsal commented Jul 29, 2021

Copy link

Copy Markdown

Member

We are missing the 3.10 backport still

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland commented Jul 30, 2021

Copy link

Copy Markdown

Contributor Author

We are missing the 3.10 backport still

Doh, I'll get to it after morning coffee :)

All reactions

Sorry, something went wrong.

Uh oh!

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

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request

Jul 30, 2021


          [3.10] bpo-31746: Prevent segfaults when sqlite3.Connection is uninit…

e72a5d8

…ialised (pythonGH-27431).

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

bedevere-bot commented Jul 30, 2021

Copy link

Copy Markdown

GH-27472 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-bot removed the needs backport to 3.10 label

Jul 30, 2021

erlend-aasland mentioned this pull request

Jul 30, 2021

bpo-31746: Fixed Segfaults in the sqlite module when uninitialized. #3946

Closed

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request

Jul 30, 2021


          Merge branch 'main' into sqlite-serialize

a22bb3d

Sync with main bco. pythonGH-27431

ambv pushed a commit that referenced this pull request

Jul 30, 2021


          [3.10] bpo-31746: Prevent segfaults when sqlite3.Connection is uninit…

0cb470e

…ialised (GH-27431). (GH-27472)

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>

smcv mentioned this pull request

Jan 5, 2022

bpo-43137: Revert "webbrowser: Don't run gvfs-open on GNOME" #30417

Merged

Reviewers

pablogsal

pablogsal approved these changes

berkerpeksag

Awaiting requested review from berkerpeksag

Uh oh!

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

Assignees

pablogsal

Labels

awaiting merge skip news

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

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

erlend-aasland commented Jul 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland Jul 28, 2021

Choose a reason for hiding this comment

Uh oh!

erlend-aasland commented Jul 29, 2021

Uh oh!

miss-islington commented Jul 29, 2021

Uh oh!

miss-islington commented Jul 29, 2021

Uh oh!

miss-islington commented Jul 29, 2021

Uh oh!

miss-islington commented Jul 29, 2021

Uh oh!

pablogsal commented Jul 29, 2021

Uh oh!

erlend-aasland commented Jul 29, 2021

Uh oh!

pablogsal commented Jul 29, 2021

Uh oh!

erlend-aasland commented Jul 29, 2021

Uh oh!

bedevere-bot commented Jul 29, 2021

Uh oh!

pablogsal commented Jul 29, 2021

Uh oh!

erlend-aasland commented Jul 30, 2021

Uh oh!

bedevere-bot commented Jul 30, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

erlend-aasland commented Jul 28, 2021 •

edited

Loading

Uh oh!

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