Activity

1. cyang1 commented on Apr 28, 2018

   

   cyang1mannequin

   on Apr 28, 2018

   MannequinAuthor

   More actions

   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,))
   
2. cyang1

   added

   3.7 (EOL)end of lifeend of life

   3.8 (EOL)end of lifeend of life

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Apr 28, 2018
3. benjaminp commented on Mar 10, 2019

   

   benjaminp

   on Mar 10, 2019

   Contributor

   More actions

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

   closed this as completedon Mar 10, 2019
5. benjaminp commented on Mar 10, 2019

   

   benjaminp

   on Mar 10, 2019

   Contributor

   More actions

   Sorry, wrong bug.
6. benjaminp

   reopened this on Mar 10, 2019
7. MaximeBelanger

   added

   3.9 (EOL)end of lifeend of life

   and removed

   3.7 (EOL)end of lifeend of life

   on Sep 4, 2019
8. animalize commented on May 10, 2021

   

   animalizemannequin

   on May 10, 2021

   Mannequin

   More actions

   Erlend, please take a look at this bug.
9. erlend-aasland commented on May 10, 2021

   

   erlend-aasland

   on May 10, 2021

   Contributor

   More actions

   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.
10. animalize commented on Nov 28, 2021

    

    animalizemannequin

    on Nov 28, 2021

    Mannequin

    More actions

    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;
11. animalize commented on Nov 28, 2021

    

    animalizemannequin

    on Nov 28, 2021

    Mannequin

    More actions

    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.
12. erlend-aasland commented on Nov 28, 2021

    

    erlend-aasland

    on Nov 28, 2021

    Contributor

    More actions

    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.
13. erlend-aasland commented on Jan 3, 2022

    

    erlend-aasland

    on Jan 3, 2022

    Contributor

    More actions

    #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.
14. erlend-aasland

    closed this as completedon Jan 3, 2022
15. ezio-melotti

    transferred this issue fromon Apr 10, 2022