Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lciti opened this issue Jun 9, 2016 · 1 comment

Comments

@lciti
Copy link

lciti commented Jun 9, 2016

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
Copy link
Author

lciti commented Jun 10, 2016

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant