gh-139327: consolidate sqlite3_finalize and sqlite3_reset usages (GH-139329)

Author: picnixz

Modules/_sqlite/blob.c

@@ -118,7 +118,9 @@ static void
 blob_seterror(pysqlite_Blob *self, int rc)
 {
     assert(self->connection != NULL);
-    set_error_from_db(self->connection->state, self->connection->db);
+    assert(rc != SQLITE_OK);
+    set_error_from_code(self->connection->state, rc);
+    assert(PyErr_Occurred());
 }
 
 static PyObject *

Modules/_sqlite/cursor.c

@@ -58,6 +58,41 @@ check_cursor_locked(pysqlite_Cursor *cur)
     return 1;
 }
 
+static pysqlite_state *
+get_module_state_by_cursor(pysqlite_Cursor *cursor)
+{
+    if (cursor->connection != NULL && cursor->connection->state != NULL) {
+        return cursor->connection->state;
+    }
+    return pysqlite_get_state_by_type(Py_TYPE(cursor));
+}
+
+static void
+cursor_sqlite3_internal_error(pysqlite_Cursor *cursor,
+                              const char *error_message,
+                              int chain_exceptions)
+{
+    pysqlite_state *state = get_module_state_by_cursor(cursor);
+    if (chain_exceptions) {
+        assert(PyErr_Occurred());
+        PyObject *exc = PyErr_GetRaisedException();
+        PyErr_SetString(state->InternalError, error_message);
+        _PyErr_ChainExceptions1(exc);
+    }
+    else {
+        assert(!PyErr_Occurred());
+        PyErr_SetString(state->InternalError, error_message);
+   }
+}
+
+static void
+cursor_cannot_reset_stmt_error(pysqlite_Cursor *cursor, int chain_exceptions)
+{
+    cursor_sqlite3_internal_error(cursor,
+                                  "cannot reset statement",
+                                  chain_exceptions);
+}
+
 /*[clinic input]
 module _sqlite3
 class _sqlite3.Cursor "pysqlite_Cursor *" "clinic_state()->CursorType"
@@ -173,8 +208,12 @@ cursor_clear(PyObject *op)
     Py_CLEAR(self->row_factory);
     if (self->statement) {
         /* Reset the statement if the user has not closed the cursor */
-        stmt_reset(self->statement);
+        int rc = stmt_reset(self->statement);
         Py_CLEAR(self->statement);
+        if (rc != SQLITE_OK) {
+            cursor_cannot_reset_stmt_error(self, 0);
+            PyErr_FormatUnraisable("Exception ignored in cursor_clear()");
+        }
     }
 
     return 0;
@@ -837,7 +876,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
 
     if (self->statement) {
         // Reset pending statements on this cursor.
-        (void)stmt_reset(self->statement);
+        if (stmt_reset(self->statement) != SQLITE_OK) {
+            goto reset_failure;
+        }
     }
 
     PyObject *stmt = get_statement_from_cache(self, operation);
@@ -861,7 +902,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
         }
     }
 
-    (void)stmt_reset(self->statement);
+    if (stmt_reset(self->statement) != SQLITE_OK) {
+        goto reset_failure;
+    }
     self->rowcount = self->statement->is_dml ? 0L : -1L;
 
     /* We start a transaction implicitly before a DML statement.
@@ -943,7 +986,9 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
             if (self->statement->is_dml) {
                 self->rowcount += (long)sqlite3_changes(self->connection->db);
             }
-            stmt_reset(self->statement);
+            if (stmt_reset(self->statement) != SQLITE_OK) {
+                goto reset_failure;
+            }
         }
         Py_XDECREF(parameters);
     }
@@ -968,8 +1013,15 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
 
     if (PyErr_Occurred()) {
         if (self->statement) {
-            (void)stmt_reset(self->statement);
+            sqlite3 *db = sqlite3_db_handle(self->statement->st);
+            int sqlite3_state = sqlite3_errcode(db);
+            // stmt_reset() may return a previously set exception,
+            // either triggered because of Python or sqlite3.
+            rc = stmt_reset(self->statement);
             Py_CLEAR(self->statement);
+            if (sqlite3_state == SQLITE_OK && rc != SQLITE_OK) {
+                cursor_cannot_reset_stmt_error(self, 1);
+            }
         }
         self->rowcount = -1L;
         return NULL;
@@ -978,6 +1030,20 @@ _pysqlite_query_execute(pysqlite_Cursor* self, int multiple, PyObject* operation
         Py_CLEAR(self->statement);
     }
     return Py_NewRef((PyObject *)self);
+
+reset_failure:
+    /* suite to execute when stmt_reset() failed and no exception is set */
+    assert(!PyErr_Occurred());
+
+    Py_XDECREF(parameters);
+    Py_XDECREF(parameters_iter);
+    Py_XDECREF(parameters_list);
+
+    self->locked = 0;
+    self->rowcount = -1L;
+    Py_CLEAR(self->statement);
+    cursor_cannot_reset_stmt_error(self, 0);
+    return NULL;
 }
 
 /*[clinic input]
@@ -1120,14 +1186,20 @@ pysqlite_cursor_iternext(PyObject *op)
         if (self->statement->is_dml) {
             self->rowcount = (long)sqlite3_changes(self->connection->db);
         }
-        (void)stmt_reset(self->statement);
+        rc = stmt_reset(self->statement);
         Py_CLEAR(self->statement);
+        if (rc != SQLITE_OK) {
+            goto reset_failure;
+        }
     }
     else if (rc != SQLITE_ROW) {
-        set_error_from_db(self->connection->state, self->connection->db);
-        (void)stmt_reset(self->statement);
+        rc = set_error_from_db(self->connection->state, self->connection->db);
+        int reset_rc = stmt_reset(self->statement);
         Py_CLEAR(self->statement);
         Py_DECREF(row);
+        if (rc == SQLITE_OK && reset_rc != SQLITE_OK) {
+            goto reset_failure;
+        }
         return NULL;
     }
     if (!Py_IsNone(self->row_factory)) {
@@ -1137,6 +1209,10 @@ pysqlite_cursor_iternext(PyObject *op)
         Py_SETREF(row, new_row);
     }
     return row;
+
+reset_failure:
+    cursor_cannot_reset_stmt_error(self, 0);
+    return NULL;
 }
 
 /*[clinic input]
@@ -1291,8 +1367,15 @@ pysqlite_cursor_close_impl(pysqlite_Cursor *self)
     }
 
     if (self->statement) {
-        (void)stmt_reset(self->statement);
+        int rc = stmt_reset(self->statement);
+        // Force self->statement to be NULL even if stmt_reset() may have
+        // failed to avoid a possible double-free if someone calls close()
+        // twice as a leak here would be better than a double-free.
         Py_CLEAR(self->statement);
+        if (rc != SQLITE_OK) {
+            cursor_cannot_reset_stmt_error(self, 0);
+            return NULL;
+        }
     }
 
     self->closed = 1;

Modules/_sqlite/statement.c

@@ -103,7 +103,12 @@ pysqlite_statement_create(pysqlite_Connection *connection, PyObject *sql)
     return self;
 
 error:
-    (void)sqlite3_finalize(stmt);
+    assert(PyErr_Occurred());
+    if (sqlite3_finalize(stmt) != SQLITE_OK) {
+        PyObject *exc = PyErr_GetRaisedException();
+        PyErr_SetString(connection->InternalError, "cannot finalize statement");
+        _PyErr_ChainExceptions1(exc);
+    }
     return NULL;
 }
 
@@ -114,10 +119,16 @@ stmt_dealloc(PyObject *op)
     PyTypeObject *tp = Py_TYPE(self);
     PyObject_GC_UnTrack(op);
     if (self->st) {
+        int rc;
         Py_BEGIN_ALLOW_THREADS
-        sqlite3_finalize(self->st);
+        rc = sqlite3_finalize(self->st);
         Py_END_ALLOW_THREADS
-        self->st = 0;
+        self->st = NULL;
+        if (rc != SQLITE_OK) {
+            pysqlite_state *state = PyType_GetModuleState(Py_TYPE(op));
+            PyErr_SetString(state->InternalError, "cannot finalize statement");
+            PyErr_FormatUnraisable("Exception ignored in stmt_dealloc()");
+        }
     }
     tp->tp_free(self);
     Py_DECREF(tp);

Modules/_sqlite/util.c

@@ -135,21 +135,22 @@ set_error_from_code(pysqlite_state *state, int code)
 /**
  * Checks the SQLite error code and sets the appropriate DB-API exception.
  */
-void
+int
 set_error_from_db(pysqlite_state *state, sqlite3 *db)
 {
     int errorcode = sqlite3_errcode(db);
     PyObject *exc_class = get_exception_class(state, errorcode);
     if (exc_class == NULL) {
         // No new exception need be raised.
-        return;
+        return SQLITE_OK;
     }
 
     /* Create and set the exception. */
     int extended_errcode = sqlite3_extended_errcode(db);
     // sqlite3_errmsg() always returns an UTF-8 encoded message
     const char *errmsg = sqlite3_errmsg(db);
     raise_exception(exc_class, extended_errcode, errmsg);
+    return errorcode;
 }
 
 #ifdef WORDS_BIGENDIAN

Modules/_sqlite/util.h

@@ -31,7 +31,7 @@
 /**
  * Checks the SQLite error code and sets the appropriate DB-API exception.
  */
-void set_error_from_db(pysqlite_state *state, sqlite3 *db);
+int set_error_from_db(pysqlite_state *state, sqlite3 *db);
 void set_error_from_code(pysqlite_state *state, int code);
 
 sqlite_int64 _pysqlite_long_as_int64(PyObject * value);