bpo-44958: Only reset `sqlite3` statements when needed (GH-27844)
diff --git a/Lib/sqlite3/test/test_regression.py b/Lib/sqlite3/test/test_regression.py
index f75e48f..d41f118 100644
--- a/Lib/sqlite3/test/test_regression.py
+++ b/Lib/sqlite3/test/test_regression.py
@@ -124,13 +124,14 @@
"""
SELECT = "select * from foo"
con = sqlite.connect(":memory:",detect_types=sqlite.PARSE_DECLTYPES)
- con.execute("create table foo(bar timestamp)")
- con.execute("insert into foo(bar) values (?)", (datetime.datetime.now(),))
- con.execute(SELECT).close()
- con.execute("drop table foo")
- con.execute("create table foo(bar integer)")
- con.execute("insert into foo(bar) values (5)")
- con.execute(SELECT).close()
+ cur = con.cursor()
+ cur.execute("create table foo(bar timestamp)")
+ cur.execute("insert into foo(bar) values (?)", (datetime.datetime.now(),))
+ cur.execute(SELECT)
+ cur.execute("drop table foo")
+ cur.execute("create table foo(bar integer)")
+ cur.execute("insert into foo(bar) values (5)")
+ cur.execute(SELECT)
def test_bind_mutating_list(self):
# Issue41662: Crash when mutate a list of parameters during iteration.
@@ -435,6 +436,40 @@
val = cur.fetchone()[0]
self.assertEqual(val, b'')
+ def test_table_lock_cursor_replace_stmt(self):
+ con = sqlite.connect(":memory:")
+ cur = con.cursor()
+ cur.execute("create table t(t)")
+ cur.executemany("insert into t values(?)", ((v,) for v in range(5)))
+ con.commit()
+ cur.execute("select t from t")
+ cur.execute("drop table t")
+ con.commit()
+
+ def test_table_lock_cursor_dealloc(self):
+ con = sqlite.connect(":memory:")
+ con.execute("create table t(t)")
+ con.executemany("insert into t values(?)", ((v,) for v in range(5)))
+ con.commit()
+ cur = con.execute("select t from t")
+ del cur
+ con.execute("drop table t")
+ con.commit()
+
+ def test_table_lock_cursor_non_readonly_select(self):
+ con = sqlite.connect(":memory:")
+ con.execute("create table t(t)")
+ con.executemany("insert into t values(?)", ((v,) for v in range(5)))
+ con.commit()
+ def dup(v):
+ con.execute("insert into t values(?)", (v,))
+ return
+ con.create_function("dup", 1, dup)
+ cur = con.execute("select dup(t) from t")
+ del cur
+ con.execute("drop table t")
+ con.commit()
+
if __name__ == "__main__":
unittest.main()
diff --git a/Modules/_sqlite/cursor.c b/Modules/_sqlite/cursor.c
index 38ccdcf..9bac607 100644
--- a/Modules/_sqlite/cursor.c
+++ b/Modules/_sqlite/cursor.c
@@ -104,12 +104,7 @@
Py_CLEAR(self->row_cast_map);
Py_CLEAR(self->lastrowid);
Py_CLEAR(self->row_factory);
- if (self->statement) {
- /* Reset the statement if the user has not closed the cursor */
- pysqlite_statement_reset(self->statement);
- Py_CLEAR(self->statement);
- }
-
+ Py_CLEAR(self->statement);
return 0;
}
@@ -121,6 +116,14 @@
if (self->in_weakreflist != NULL) {
PyObject_ClearWeakRefs((PyObject*)self);
}
+ if (self->statement) {
+ /* A SELECT query will lock the affected database table(s), so we need
+ * to reset the statement to unlock the database before disappearing */
+ sqlite3_stmt *stmt = self->statement->st;
+ if (sqlite3_stmt_readonly(stmt)) {
+ pysqlite_statement_reset(self->statement);
+ }
+ }
tp->tp_clear((PyObject *)self);
tp->tp_free(self);
Py_DECREF(tp);
@@ -515,18 +518,19 @@
}
}
- if (self->statement != NULL) {
- /* There is an active statement */
- pysqlite_statement_reset(self->statement);
- }
-
/* reset description and rowcount */
Py_INCREF(Py_None);
Py_SETREF(self->description, Py_None);
self->rowcount = 0L;
if (self->statement) {
- (void)pysqlite_statement_reset(self->statement);
+ /* A SELECT query will lock the affected database table(s), so we need
+ * to reset the statement to unlock the database before switching
+ * statements */
+ sqlite3_stmt *stmt = self->statement->st;
+ if (sqlite3_stmt_readonly(stmt)) {
+ pysqlite_statement_reset(self->statement);
+ }
}
PyObject *stmt = get_statement_from_cache(self, operation);
@@ -549,8 +553,6 @@
goto error;
}
}
-
- pysqlite_statement_reset(self->statement);
pysqlite_statement_mark_dirty(self->statement);
/* We start a transaction implicitly before a DML statement.
@@ -570,6 +572,7 @@
break;
}
+ pysqlite_statement_reset(self->statement);
pysqlite_statement_mark_dirty(self->statement);
pysqlite_statement_bind_parameters(state, self->statement, parameters);
@@ -587,7 +590,6 @@
PyErr_Clear();
}
}
- (void)pysqlite_statement_reset(self->statement);
_pysqlite_seterror(state, self->connection->db);
goto error;
}
@@ -646,13 +648,9 @@
}
if (rc == SQLITE_DONE && !multiple) {
- pysqlite_statement_reset(self->statement);
Py_CLEAR(self->statement);
}
- if (multiple) {
- pysqlite_statement_reset(self->statement);
- }
Py_XDECREF(parameters);
}
@@ -804,7 +802,6 @@
sqlite3_stmt *stmt = self->statement->st;
assert(stmt != NULL);
if (sqlite3_data_count(stmt) == 0) {
- (void)pysqlite_statement_reset(self->statement);
Py_CLEAR(self->statement);
return NULL;
}
@@ -815,7 +812,7 @@
}
int rc = pysqlite_step(stmt);
if (rc == SQLITE_DONE) {
- (void)pysqlite_statement_reset(self->statement);
+ Py_CLEAR(self->statement);
}
else if (rc != SQLITE_ROW) {
(void)_pysqlite_seterror(self->connection->state,
@@ -985,11 +982,7 @@
return NULL;
}
- if (self->statement) {
- (void)pysqlite_statement_reset(self->statement);
- Py_CLEAR(self->statement);
- }
-
+ Py_CLEAR(self->statement);
self->closed = 1;
Py_RETURN_NONE;
diff --git a/Modules/_sqlite/statement.c b/Modules/_sqlite/statement.c
index b20c91d..3016fe5 100644
--- a/Modules/_sqlite/statement.c
+++ b/Modules/_sqlite/statement.c
@@ -360,23 +360,31 @@
}
}
-int pysqlite_statement_reset(pysqlite_Statement* self)
+void
+pysqlite_statement_reset(pysqlite_Statement *self)
{
- int rc;
-
- rc = SQLITE_OK;
-
- if (self->in_use && self->st) {
- Py_BEGIN_ALLOW_THREADS
- rc = sqlite3_reset(self->st);
- Py_END_ALLOW_THREADS
-
- if (rc == SQLITE_OK) {
- self->in_use = 0;
- }
+ sqlite3_stmt *stmt = self->st;
+ if (stmt == NULL || self->in_use == 0) {
+ return;
}
- return rc;
+#if SQLITE_VERSION_NUMBER >= 3020000
+ /* Check if the statement has been run (that is, sqlite3_step() has been
+ * called at least once). Third parameter is non-zero in order to reset the
+ * run count. */
+ if (sqlite3_stmt_status(stmt, SQLITE_STMTSTATUS_RUN, 1) == 0) {
+ return;
+ }
+#endif
+
+ int rc;
+ Py_BEGIN_ALLOW_THREADS
+ rc = sqlite3_reset(stmt);
+ Py_END_ALLOW_THREADS
+
+ if (rc == SQLITE_OK) {
+ self->in_use = 0;
+ }
}
void pysqlite_statement_mark_dirty(pysqlite_Statement* self)
diff --git a/Modules/_sqlite/statement.h b/Modules/_sqlite/statement.h
index b901c43..cce81ed 100644
--- a/Modules/_sqlite/statement.h
+++ b/Modules/_sqlite/statement.h
@@ -44,7 +44,7 @@
pysqlite_Statement *self,
PyObject *parameters);
-int pysqlite_statement_reset(pysqlite_Statement* self);
+void pysqlite_statement_reset(pysqlite_Statement *self);
void pysqlite_statement_mark_dirty(pysqlite_Statement* self);
int pysqlite_statement_setup_types(PyObject *module);