bpo-44958: Only reset sqlite3 statements when needed (GH-27844)

Author: Erlend Egeberg Aasland

Lib/sqlite3/test/test_regression.py

@@ -124,13 +124,14 @@ def test_type_map_usage(self):
         """
         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 @@ def test_return_empty_bytestring(self):
         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()

Modules/_sqlite/cursor.c

@@ -104,12 +104,7 @@ cursor_clear(pysqlite_Cursor *self)
     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 @@ cursor_dealloc(pysqlite_Cursor *self)
     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 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
         }
     }
 
-    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 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
             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 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
             break;
         }
 
+        pysqlite_statement_reset(self->statement);
         pysqlite_statement_mark_dirty(self->statement);
 
         pysqlite_statement_bind_parameters(state, self->statement, parameters);
@@ -587,7 +590,6 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
                     PyErr_Clear();
                 }
             }
-            (void)pysqlite_statement_reset(self->statement);
             _pysqlite_seterror(state, self->connection->db);
             goto error;
         }
@@ -646,13 +648,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
         }
 
         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 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self)
     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 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self)
     }
     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 @@ pysqlite_cursor_close_impl(pysqlite_Cursor *self, PyTypeObject *cls)
         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;

Modules/_sqlite/statement.c

@@ -360,23 +360,31 @@ pysqlite_statement_bind_parameters(pysqlite_state *state,
     }
 }
 
-int pysqlite_statement_reset(pysqlite_Statement* self)
+void
+pysqlite_statement_reset(pysqlite_Statement *self)
 {
-    int rc;
+    sqlite3_stmt *stmt = self->st;
+    if (stmt == NULL || self->in_use == 0) {
+        return;
+    }
 
-    rc = SQLITE_OK;
+#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
 
-    if (self->in_use && self->st) {
-        Py_BEGIN_ALLOW_THREADS
-        rc = sqlite3_reset(self->st);
-        Py_END_ALLOW_THREADS
+    int rc;
+    Py_BEGIN_ALLOW_THREADS
+    rc = sqlite3_reset(stmt);
+    Py_END_ALLOW_THREADS
 
-        if (rc == SQLITE_OK) {
-            self->in_use = 0;
-        }
+    if (rc == SQLITE_OK) {
+        self->in_use = 0;
     }
-
-    return rc;
 }
 
 void pysqlite_statement_mark_dirty(pysqlite_Statement* self)

Modules/_sqlite/statement.h

@@ -44,7 +44,7 @@ void pysqlite_statement_bind_parameters(pysqlite_state *state,
                                         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);