[Python-checkins] [3.9] bpo-27334: roll back transaction if sqlite3 context manager fails to commit (GH-26202) (GH-27944)
pablogsal
webhook-mailer at python.org
Wed Aug 25 15:02:34 EDT 2021
https://github.com/python/cpython/commit/52702e8ba0d777ac92ec36038213976545c4759e
commit: 52702e8ba0d777ac92ec36038213976545c4759e
branch: 3.9
author: Erlend Egeberg Aasland <erlend.aasland at innova.no>
committer: pablogsal <Pablogsal at gmail.com>
date: 2021-08-25T20:02:25+01:00
summary:
[3.9] bpo-27334: roll back transaction if sqlite3 context manager fails to commit (GH-26202) (GH-27944)
files:
A Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst
M Lib/sqlite3/test/dbapi.py
M Modules/_sqlite/connection.c
diff --git a/Lib/sqlite3/test/dbapi.py b/Lib/sqlite3/test/dbapi.py
index c43eb60e63b258..38e9fbd9d1dd60 100644
--- a/Lib/sqlite3/test/dbapi.py
+++ b/Lib/sqlite3/test/dbapi.py
@@ -21,11 +21,13 @@
# misrepresented as being the original software.
# 3. This notice may not be removed or altered from any source distribution.
+import subprocess
import threading
import unittest
import sqlite3 as sqlite
+import sys
-from test.support import TESTFN, unlink
+from test.support import SHORT_TIMEOUT, TESTFN, unlink
class ModuleTests(unittest.TestCase):
@@ -954,6 +956,77 @@ def CheckOnConflictReplace(self):
self.assertEqual(self.cu.fetchall(), [('Very different data!', 'foo')])
+class MultiprocessTests(unittest.TestCase):
+ CONNECTION_TIMEOUT = SHORT_TIMEOUT / 1000. # Defaults to 30 ms
+
+ def tearDown(self):
+ unlink(TESTFN)
+
+ def test_ctx_mgr_rollback_if_commit_failed(self):
+ # bpo-27334: ctx manager does not rollback if commit fails
+ SCRIPT = f"""if 1:
+ import sqlite3
+ def wait():
+ print("started")
+ assert "database is locked" in input()
+
+ cx = sqlite3.connect("{TESTFN}", timeout={self.CONNECTION_TIMEOUT})
+ cx.create_function("wait", 0, wait)
+ with cx:
+ cx.execute("create table t(t)")
+ try:
+ # execute two transactions; both will try to lock the db
+ cx.executescript('''
+ -- start a transaction and wait for parent
+ begin transaction;
+ select * from t;
+ select wait();
+ rollback;
+
+ -- start a new transaction; would fail if parent holds lock
+ begin transaction;
+ select * from t;
+ rollback;
+ ''')
+ finally:
+ cx.close()
+ """
+
+ # spawn child process
+ proc = subprocess.Popen(
+ [sys.executable, "-c", SCRIPT],
+ encoding="utf-8",
+ bufsize=0,
+ stdin=subprocess.PIPE,
+ stdout=subprocess.PIPE,
+ )
+ self.addCleanup(proc.communicate)
+
+ # wait for child process to start
+ self.assertEqual("started", proc.stdout.readline().strip())
+
+ cx = sqlite.connect(TESTFN, timeout=self.CONNECTION_TIMEOUT)
+ try: # context manager should correctly release the db lock
+ with cx:
+ cx.execute("insert into t values('test')")
+ except sqlite.OperationalError as exc:
+ proc.stdin.write(str(exc))
+ else:
+ proc.stdin.write("no error")
+ finally:
+ cx.close()
+
+ # terminate child process
+ self.assertIsNone(proc.returncode)
+ try:
+ proc.communicate(input="end", timeout=SHORT_TIMEOUT)
+ except subprocess.TimeoutExpired:
+ proc.kill()
+ proc.communicate()
+ raise
+ self.assertEqual(proc.returncode, 0)
+
+
def suite():
module_suite = unittest.makeSuite(ModuleTests, "Check")
connection_suite = unittest.makeSuite(ConnectionTests, "Check")
@@ -965,10 +1038,11 @@ def suite():
closed_cur_suite = unittest.makeSuite(ClosedCurTests, "Check")
on_conflict_suite = unittest.makeSuite(SqliteOnConflictTests, "Check")
uninit_con_suite = unittest.makeSuite(UninitialisedConnectionTests)
+ multiproc_con_suite = unittest.makeSuite(MultiprocessTests)
return unittest.TestSuite((
module_suite, connection_suite, cursor_suite, thread_suite,
constructor_suite, ext_suite, closed_con_suite, closed_cur_suite,
- on_conflict_suite, uninit_con_suite,
+ on_conflict_suite, uninit_con_suite, multiproc_con_suite,
))
def test():
diff --git a/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst b/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst
new file mode 100644
index 00000000000000..dc0cdf33ec5acf
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-05-18-00-17-21.bpo-27334.32EJZi.rst
@@ -0,0 +1,2 @@
+The :mod:`sqlite3` context manager now performs a rollback (thus releasing the
+database lock) if commit failed. Patch by Luca Citi and Erlend E. Aasland.
diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
index 4806a12789cd19..0949e8d408e620 100644
--- a/Modules/_sqlite/connection.c
+++ b/Modules/_sqlite/connection.c
@@ -1771,22 +1771,36 @@ pysqlite_connection_enter(pysqlite_Connection* self, PyObject* args)
static PyObject *
pysqlite_connection_exit(pysqlite_Connection* self, PyObject* args)
{
- PyObject* exc_type, *exc_value, *exc_tb;
- const char* method_name;
- PyObject* result;
-
+ PyObject *exc_type, *exc_value, *exc_tb;
if (!PyArg_ParseTuple(args, "OOO", &exc_type, &exc_value, &exc_tb)) {
return NULL;
}
+ int commit = 0;
+ PyObject *result;
if (exc_type == Py_None && exc_value == Py_None && exc_tb == Py_None) {
- method_name = "commit";
- } else {
- method_name = "rollback";
+ commit = 1;
+ result = pysqlite_connection_commit(self, NULL);
}
-
- result = PyObject_CallMethod((PyObject*)self, method_name, NULL);
- if (!result) {
+ else {
+ result = pysqlite_connection_rollback(self, NULL);
+ }
+
+ if (result == NULL) {
+ if (commit) {
+ /* Commit failed; try to rollback in order to unlock the database.
+ * If rollback also fails, chain the exceptions. */
+ PyObject *exc, *val, *tb;
+ PyErr_Fetch(&exc, &val, &tb);
+ result = pysqlite_connection_rollback(self, NULL);
+ if (result == NULL) {
+ _PyErr_ChainExceptions(exc, val, tb);
+ }
+ else {
+ Py_DECREF(result);
+ PyErr_Restore(exc, val, tb);
+ }
+ }
return NULL;
}
Py_DECREF(result);
More information about the Python-checkins
mailing list