Provision against Murphy's lock

[edzard]

This is about the deadlock as described in http://lists.initd.org/pipermail/pysqlite/2006-October/000783.html. It is not a high-priority issue to me but I think it may be ruled out by a simple change of steps in the processing of cursor.c. This is by moving the begin of a transaction down, to after the statement preparations, so just before the sqlite3_step. The difference this makes for the locking situation is schematically as follws:

currently: - begin statement (issued by PySQLite) -- UNLOCKED - sqlite3_prepare -- SHARED - sqlite3_step -- RESERVED

proposed: - sqlite3_prepare -- SHARED - begin statement (issued by PySQLite) -- UNLOCKED - sqlite3_step -- RESERVED

The theory is that in the current situation two processes or process threads may arrive in the SHARED locking stage at the same time and that one of them gets deadlock upon going to RESERVED. Possibly this is not well formulated but it is the only explanation for the situation as referred to. I believe this situation may also have occured in the former version of trac.

If this is indeed a matter of moving a block of code, it may be worth adding it as a fix.

[gerhard.haering]

Looks like I should create a version where you could switch the behaviour for tests.

[edzard]

What kind of testing do you have in mind? To reproduce the lock or to test if the change is safe? I tried a script that I stll had to compare trac 10.0 and 10.2. The patch for pysqlite seems easy enough to do it myself.

You may see the test script on http://home.wanadoo.nl/italy4ever/trac/url.py. I tested three setups:

nopool - trac 10.2 without connection pool so it behaves like 10.0 as far as the locks is concerned

nopool.patch - same but with this pysqlite patch applied

normal - standard trac and pysqlite

The output files may also be seen at http://home.wanadoo.nl/italy4ever/trac . The last line of each file shows elapsed time, nr of bytes received via http and number of locking errors. Thes are:

nopool: 23 seconds 128822 bytes 3 errors

nopool.patch: 27 seconds 142982 bytes 0 errors

normal: 25 seconds 142982 bytes 0 errors

So the patch is reliable in this case. Still I wish to add that the name for this issue, "Murphy's lock" may not be well choosen. It seems more a programmatical issue. I think it can be reproduced in a single threaded programme and if you wish I'll try. Edzard

[edzard]

Just found a convenient single-thread example in http://initd.org/tracker/pysqlite/browser/pysqlite/trunk/pysqlite2/test/transactions.py/

    def CheckRaiseTimeout(self):
        self.cur1.execute("create table test(i)")
        self.cur1.execute("insert into test(i) values (5)")
        try:
            self.cur2.execute("insert into test(i) values (5)")
            self.fail("should have raised an OperationalError")
        except sqlite.OperationalError:
            pass
        except:
            self.fail("should have raised an OperationalError")

This test finishes without a commit. Try add con.commit () at the bottom and find that now con (nr 1) hits a lock. (and this is really fixed by my patch)

[edzard]

cursor.c patch for pysqlite-3.3.10

[gh]

I applied the patch in SVN and added a testcase like yours. Thanks again!

[christian.boos]

(was committed as r403, which means it's in 2.3.5)