bug on commit()

[acortiana]

I'm using sqlitedict version 1.6.0 and Python version 3.6.9
If I do this

mydict = SqliteDict('./my_db.sqlite', autocommit=False)
mydict['some_key'] = 'some_value'
mydict.commit()

and the commit fails for some reason, the
mydict.commit()
line hangs and never returns.
I think that the problem is here, maybe a try/except is needed, with some error handling code:
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404

I tried to add a try/except and, in my case, I got this error:

2020-03-06 14:56:45 ERROR    Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/sqlitedict.py", line 405, in run
    conn.commit()
sqlite3.OperationalError: database is locked

[jonchun]

I just tested the same code and it works perfectly fine on my end.

sqlite3.OperationalError: database is locked

is most likely being thrown because you have a different process/thread trying to commit to the same sqlite db at the same time. Try completely restarting your computer (to make sure any orphaned/backgrounded tasks get killed) and see if you can reproduce the issue consistently.

Additionally see if you can reproduce the issue even after changing the db file name so you're 100% sure nothing else can be writing to the db

[acortiana]

The problem is not to understand why I'm getting the exception.
In my use case, I have a lot of processes accessing the DB in the same time, and this is by design.

The problem is that sqlitedict is not responding in the right way when there is an exception on the underlying commit operation.
To do a test in the right way, you have to make the underlying commit
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404
raise an exception.
IMHO, the expected behavior is to make that exception "propagate" from the thread to the main process and be raised by
mydict.commit()
call

The current behavior is to hang indefinitely without ever returning and this cannot be the right behavior, right?

[shantanoo]

Quick fix could be adding timeout to sqlite connection.

https://stackoverflow.com/questions/2598801/python-sqlite-database-locked-despite-large-timeouts#3659261

[ed2050]

Was anything ever done to fix this? The "quick fix" doesn't appear possible because there doesn't seem to be a way to pass sqlite connection params to sqlitedict. The connection happens inside sqlitedict.

Maybe what's needed is another arg to sqlitedict constructor. Something like (dict) connect_params, a dictionary of arguments for sqlite3.connect. Then pass it to connect as kwargs.

I don't see any code in sqlitedict.commit () that would swallow an exception. My guess is that acortiana did something more besides just wrapping conn.commit () in try / except. Although it seems he's using the multithread class based on line number, so perhaps it's a thread issue (exception gets raised and silently swallowed, or error message output is ignored, in a secondary thread, but error propagates all the way to the top when testing with one thread).

[acortiana]

I did this test to better demonstrate the problem:

1. added lines

if hasattr(self, 'test123'):
    raise NameError('HiThere')
self.test123 = 1

just after this line:
https://github.com/RaRe-Technologies/sqlitedict/blob/39e8dced2f56498df6e0d58a0addf42297917d3f/sqlitedict.py#L404

2. run exactly this code

from sqlitedict import SqliteDict
mydict = SqliteDict('./my_db.sqlite', autocommit=False)
mydict['some_key'] = 'some_value'
mydict.commit()

I added the "hasattr" condition to skip the first commit from the "HiThere" exception.
This because when the mydict object is created, it does an implicit (first) commit operation to create the table.

Executing the code, you can see that the Thread-1 raises the exception but you can also see that mydict.commit() command remains stuck without ever returning.
IMHO the right behavior is that mydict.commit() also must raise an exception, without remaining stuck forever.