Description

Rolling back a transaction causes all statements associated with that transaction to be reset, which allows the statements to be used again from the pysqlite statement cache. This can interact with various methods on Cursor objects to cause these statements to be reset again, possibly when they are in use by a different cursor.

This appears to be very similar to bpo-10513 and bpo-23129.

# Impact
Duplicate rows will be returned. Exceptions can be raised.

Repro steps

• Cursor A executes query X
• Rollback occurs
• Cursor B executes query X
• Any of the following (and there might be other cases too):
  • Cursor A is closed
  • Cursor A is deallocated
  • Cursor A executes any other query.
• Result: Cursor B returns duplicate rows.
• Furthermore: Executing query X again afterwards raises sqlite3.InterfaceError

Possible solutions

• Similar to the solution for bpo-10513 and bpo-23129, we could remove pysqlite_do_all_statements(self, ACTION_RESET, 1) from pysqlite_connection_rollback. This fixes the given issue, but I'm not sure what the implications are for the rest of the system.

• Do not reset self->statement in Cursor if self->reset. This is the fix we've adopted for now (through a local patch to our Python), but it's worth noting that this is rather brittle, and only works because pysqlite_do_all_statements is always called with reset_cursors = 1, and self->reset is not modified in too many places.

# Example

import sqlite3 as sqlite

if __name__ == '__main__':
    conn = sqlite.connect(":memory:")
    conn.executescript("""
        CREATE TABLE t(c);
        INSERT INTO t VALUES(0);
        INSERT INTO t VALUES(1);
        INSERT INTO t VALUES(2);
    """)

    curs = conn.cursor()
    curs.execute("BEGIN TRANSACTION")
    curs.execute("SELECT c FROM t WHERE ?", (1,))
    conn.rollback()

    # Reusing the same statement from the statement cache, which has been
    # reset by the rollback above.
    gen = conn.execute("SELECT c FROM t WHERE ?", (1,))

    # Any of the following will cause a spurious reset of the statement.
    curs.close()
    # curs.execute("SELECT 1")
    # del curs

    # Expected output: [(0,), (1,), (2,)]
    # Observed output: [(0,), (0,), (1,), (2,)]
    print(list(gen))

    # Raises `sqlite3.InterfaceError: Error binding parameter 0 - probably unsupported type.`
    conn.execute("SELECT c FROM t WHERE ?", (1,))

New changeset 738c19f by Benjamin Peterson in branch 'master':
closes bpo-33376: Update to Unicode 12.0.0. (GH-12256)
738c19f

Sorry, wrong bug.

Erlend, please take a look at this bug.

I believe the former proposed solution is the correct solution. I'm digging though the pysqlite git history (both in the original repo and in CPython), the SQLite changelogs, and different test suites to prove me wrong :) I'm using bpo-44092 to track my findings.

Since 243b6c3 (21-08-19), this bug can't be reproduced.

In pysqlite_do_all_statements(), 243b6c3 resets statements like this:

    sqlite3_stmt *stmt = NULL;
    while ((stmt = sqlite3_next_stmt(self->db, stmt))) {
        if (sqlite3_stmt_busy(stmt)) {
            (void)sqlite3_reset(stmt);
        }
    }
    
But the `pysqlite_Statement.in_use` flag is not reset.
In `_pysqlite_query_execute()` function, if `pysqlite_Statement.in_use` flag is 1, it creates a new `pysqlite_Statement` instance. So this line will use a new statement:

    gen = conn.execute("SELECT c FROM t WHERE ?", (1,))

The duplicate row is from pysqlite_Cursor.next_row before 3df0fc8 (21-08-25).

A digressive suggestion is whether it can be changed like this, and add a check for resetting statement. So that statements are not allowed to be reset by other Cursors, which may improve code robust:

    typedef struct
    {
        ...
-       int in_use;
+       pysqlite_Cursor *in_use; // points to the attached cursor
        ...
    } pysqlite_Statement;

This issue is not resolved, but was covered by a problematic behavior.
Maybe this issue will be solved in bpo-44092, I'll study that issue later.

Maybe this issue will be solved in bpo-44092

Yes, this will be resolved in bpo-44092. Hopefully I'll be able to land it sooner than later.

#70214 / bpo-44092 has removed the old workaround where pending statements were reset before a rollback. Since version 3.7.11, SQLite no longer has any issues with pending statements and rollbacks, so we remove the workaround, since we require SQLite 3.7.15 or newer in CPython.

As a side effect, pysqlite_do_all_statements() has now been removed, as it is unused code.

Following #70214, this issue is no longer an issue; I close it as fixed.

Ma Lin: if you have problems with the LRU cache, please open a new issue.