Repository navigation
2.7.13 _sqlite more prone to "database table is locked" #73192
Description
| BPO | 29006 |
|---|---|
| Nosy | @malemburg, @arigo, @larryhastings, @benjaminp, @berkerpeksag, @zware, @palaviv |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2020-04-27.03:35:58.312>
created_at = <Date 2016-12-18.21:18:59.224>
labels = ['3.7', 'invalid', 'type-bug', 'library']
title = '2.7.13 _sqlite more prone to "database table is locked"'
updated_at = <Date 2020-04-27.03:35:58.311>
user = 'https://github.com/arigo'bugs.python.org fields:
activity = <Date 2020-04-27.03:35:58.311>
actor = 'zach.ware'
assignee = 'none'
closed = True
closed_date = <Date 2020-04-27.03:35:58.312>
closer = 'zach.ware'
components = ['Library (Lib)']
creation = <Date 2016-12-18.21:18:59.224>
creator = 'arigo'
dependencies = []
files = []
hgrepos = []
issue_num = 29006
keywords = []
message_count = 17.0
messages = ['283569', '283570', '283675', '284435', '284478', '284852', '285188', '285207', '285292', '285295', '285376', '285380', '285384', '285461', '285470', '285543', '367371']
nosy_count = 10.0
nosy_names = ['lemburg', 'arigo', 'ghaering', 'larry', 'benjamin.peterson', 'python-dev', 'berker.peksag', 'zach.ware', 'Gian-Carlo Pascutto', 'palaviv']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29006'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']
Activity
arigo commentedon Dec 18, 2016
2.7.13 did a small change to the sqlite commit() method, http://bugs.python.org/issue10513, which I think is causing troubles. I noticed the problem in PyPy, which (with the same change) fails another test in Lib/sqlite3/test/regression.py, CheckTypeMapUsage(), containing this code:
The first execute() method creates a cursor; assuming it is not promptly deleted, its mere existence causes the second execute() method to fail inside Sqlite with "OperationalError: database table is locked". As a second step, I could reproduce the problem in CPython by changing the test like this:
The reason is that in the commit() done by the second execute(), we no longer reset all this connection's cursors before proceeding. But depending on the operation, Sqlite may then complain that the "table is locked" by these old cursors. In other words, this new situation introduced in 2.7.13 potentially makes a few complicated cases crash by "table is locked" on CPython, where they would work fine previously---which is bad IMHO. About PyPy, many more cases would crash, to the point that we may have no choice but not implement this 2.7.13 change at all.
arigo commentedon Dec 18, 2016
Or maybe it would be enough to change commit() so that if Sqlite fails with "table is locked", pysqlite would reset all cursors and then try again?
arigo commentedon Dec 20, 2016
Tried that, but reverted because on Windows CheckTypeMapUsage() would fail with SQLITE_MISUSE ("ProgrammingError: database table is locked"). For now PyPy will not implement this 2.7.13 change. I really suspect you can get the same problems on CPython in some cases, as described.
berkerpeksag commentedon Jan 1, 2017
Thanks for the report. I can also reproduce it with 3.5+. We may want to revert 030e100f048a for the next 2.7 and 3.5 releases if we can't come up with a solution.
malemburg commentedon Jan 2, 2017
As general comment, I think you'd have to check which operation modes SQLite supports for the case of a transaction commit/rollback in the light of open cursors.
ODBC defines the following cases and each data source can specify a different behavior (https://msdn.microsoft.com/en-us/library/ms711769%28v=vs.85%29.aspx):
All cursors are closed, and access plans for prepared statements on that connection are deleted.
All cursors are closed, and access plans for prepared statements on that connection remain intact.
All cursors remain open, and access plans for prepared statements on that connection remain intact.
The Python DB-API does not specify any particular behavior (since this depends on the database backend), so I guess pysqlite3 should implement whatever SQLite supports per default in such cases (or make this configurable).
BTW: It is quite common for databases to support the second mode of operation:
since this allows pooling cursors to cache often used access plans.
larryhastings commentedon Jan 6, 2017
FYI I'm keeping an eye on this for possible cherry-picking into 3.5.3 final, depending on the resolution. Reverting 030e100f048a work for me, assuming that's a reasonable solution.
larryhastings commentedon Jan 11, 2017
Ping. Hoping to resolve this in time for 3.5.3, which I tag in about four days.
arigo commentedon Jan 11, 2017
If I had a say, I would indeed revert 030e100f048a (2.7 branch) and 81f614dd8136 (3.5 branch) as well as forward-port the revert to 3.6 and trunk. Then we wait for someone that really knows why the change was done in the first place.
python-dev commentedon Jan 12, 2017
New changeset dd13098a5dc2 by Benjamin Peterson in branch '2.7':
revert 030e100f048a (bpo-29006, bpo-10513)
https://hg.python.org/cpython/rev/dd13098a5dc2
Gian-CarloPascutto commentedon Jan 12, 2017
Python 2.7 had a regression compared to 2.6 where a SELECT after a COMMIT would silently return the wrong data:
http://bugs.python.org/issue23129
http://bugs.python.org/issue10513
Neither exiting with a locked error nor producing the wrong data are particularly appealing results. Did 2.6 pass those sets that caused the attempted fix to get backed out?
arigo commentedon Jan 13, 2017
Gian-Carlo is right: I can modify the 2.6 tests in the same way as I described, and then I get the same error with python2.6. So it seems that all of 2.6 was prone to the same issue, and it was never found, but went away in 2.7 accidentally. That seems to mean that reverting 030e100f048a was not really necessary (beyond the issue with PyPy/Jython/IronPython).
According to http://bugs.python.org/issue23129 it was not a good idea to revert 030e100f048a. But I'm surprized that the test added with 030e100f048a continues to pass with the revertion. Maybe we should investigate why it does. (Again, don't rely on me for that, because I don't know sqlite.)
If 030e100f048a stays in, I'll probably figure out a hack to avoid this pitfall with PyPy.
arigo commentedon Jan 13, 2017
...hah, of course the commit dd13098a5dc2 also reverted away the new test. That test fails. Sorry about that, and feel free to redo that commit. It's just one more case in which the implicit refcounting is used, but I guess as it fixes a real issue it's a good idea anyway and should be fixed first in PyPy.
arigo commentedon Jan 13, 2017
Managed to write a patch in PyPy that seems to pass all tests including the new one, including on Windows. I know think that dd13098a5dc2 should be backed out (i.e. 030e100f048a should be kept).
Reference to the PyPy changes: https://bitbucket.org/pypy/pypy/commits/235e8a3889790042b3f148bcf04891b27f97a1fc
Maybe something similar should be added to CPython, to avoid the unexpected "database table is locked" case; but such a change should probably be done only in trunk, because the <= 2.6 experience seems to suggest it is rare enough in practice.
larryhastings commentedon Jan 14, 2017
Hoping to tag in less than 48 hours...!
arigo commentedon Jan 14, 2017
larry: unless someone else comments, I think now that the current status of 3.5.3 is fine enough (nothing was done in this branch, and the problem I describe and just fixed in PyPy can be left for later).
The revert dd13098a5dc2 needs to be itself reverted in the 2.7 branch.
python-dev commentedon Jan 16, 2017
New changeset 74a68d86569c by Benjamin Peterson in branch '2.7':
revert dd13098a5dc2 (bpo-29006, bpo-10513)
https://hg.python.org/cpython/rev/74a68d86569c
zware commentedon Apr 27, 2020
As far as I can tell from the commit history associated with this issue this seems to have not been a bug, so I'm closing it. If anyone disagrees, please reopen :)