ghaering / pysqlite

Python DB-API module for SQLite 3.
zlib License
372 stars 89 forks source link

pysqlite3 context manager not performing rollback when a database is locked elsewhere for non-DML statements #103

Open lciti opened 8 years ago

lciti commented 8 years ago

The pysqlite3 context manager does not perform a rollback when a transaction fails because the database is locked by some other process performing non-DML statements (e.g. during the sqlite3 command line .dump method).

To reproduce the problem, open a terminal and run the following:

sqlite3 /tmp/test.db 'drop table person; create table person (id integer primary key, firstname varchar)'
echo -e 'begin transaction;\nselect * from person;\n.system sleep 1000\nrollback;' | sqlite3 /tmp/test.db

Leave this shell running and run the python3 interpreter from a different shell, then type:

import sqlite3
con = sqlite3.connect('/tmp/test.db')
with con:                                                        
    con.execute("insert into person(firstname) values (?)", ("Jan",))
    pass

You should receive the following:

      1 with con:
      2         con.execute("insert into person(firstname) values (?)", ("Jan",))
----> 3         pass
      4 

OperationalError: database is locked

Without exiting python, switch back to the first shell and kill the 'echo ... | sqlite3' process. Then run:

sqlite3 /tmp/test.db .dump

you should get:

PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
/**** ERROR: (5) database is locked *****/
ROLLBACK; -- due to errors

This means that the python process never executed a rollback and is still holding the lock. To release the lock one can exit python (clearly, this is not the intended behaviour of the context manager).

I believe the reason for this problem is that the exception happened in the implicit commit that is run on exiting the context manager, rather than inside it. In fact the exception is in the pass line rather than in the execute line. This exception did not trigger a rollback because the it happened after pysqlite_connection_exit checks for exceptions.

The expected behaviour (pysqlite3 rolling back and releasing the lock) is recovered if the initial blocking process is a Data Modification Language (DML) statement, e.g.:

echo -e 'begin transaction; insert into person(firstname) values ("James");\n.system sleep 1000\nrollback;' | sqlite3 /tmp/test.db

because this raises an exception at the execute time rather than at commit time.

To fix this problem, I think the pysqlite_connection_exit function in src/connection.c should handle the case when the commit itself raises an exception, and invoke a rollback.

lciti commented 8 years ago

It looks like the following does the job:

diff --git a/src/connection.c b/src/connection.c
index 9ca3afa..e61da6f 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1578,6 +1578,10 @@ pysqlite_connection_exit(pysqlite_Connection* self, PyObject* args)

     result = PyObject_CallMethod((PyObject*)self, method_name, "");
     if (!result) {
+        result = PyObject_CallMethod((PyObject*)self, "rollback", "");
+        if (result) {
+            Py_DECREF(result);
+        }
         return NULL;
     }
     Py_DECREF(result);