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

deadlock with threads #820

Open
lazka opened this issue May 8, 2023 · 5 comments · Fixed by #839
Open

deadlock with threads #820

lazka opened this issue May 8, 2023 · 5 comments · Fixed by #839
Labels
Milestone

Comments

@lazka
Copy link

lazka commented May 8, 2023

See the reproducer here: https://github.com/lazka/test-cache

This downloads small files (hosted on the test repo itself) and caches things via sqlite.
After a few runs this deadlocks.

@JWCook
Copy link
Member

JWCook commented May 15, 2023

Thanks for the reproducible example! I'm a little busier than usual right now, but I'll look into this as soon as I get the chance.

@lazka
Copy link
Author

lazka commented May 16, 2023

No problem, I worked around it for now by disabling caching in that place.

@JWCook JWCook added this to the v1.1 milestone Jun 2, 2023
@JWCook
Copy link
Member

JWCook commented Jun 5, 2023

Just FYI, I haven't forgotten about this. I have spent some time trying to debug it, but it's a tricky one.

Part of the problem was threads not releasing their locks if an error occurs within a db transaction. That part was easy enough to fix, but the actual errors are bizarre, like:

# In SQLiteDict.__setitem__()
sqlite3.DatabaseError: no more rows available
# In SQLiteDict.__getitem__()
sqlite3.DatabaseError: not an error

SQLite should be more than capable of handling the volume of concurrent reads & writes from your example, so I'm not sure what's going on there. And "not an error" is definitely a new one for me. I'll keep looking into it when I have time, though.

@JWCook
Copy link
Member

JWCook commented Jun 13, 2023

This has been an especially challenging one because there are several contributing factors, but I believe I'm close to getting this fixed.

@JWCook
Copy link
Member

JWCook commented Jun 30, 2023

@lazka Some improvements related to this are now available in v1.1.

I don't think this is completely fixed, though, since running your test still occasionally gives me some DatabaseErrors like the ones mentioned above (usually within 10-30 minutes).

Still, it's worth giving these changes a try. I'd also recommend trying out WAL mode, which you can enable with with SQLiteCache(wal=True).

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

Successfully merging a pull request may close this issue.

2 participants