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

Potential race condition fix? #82

Closed
dgutson opened this issue Jul 20, 2023 · 5 comments
Closed

Potential race condition fix? #82

dgutson opened this issue Jul 20, 2023 · 5 comments

Comments

@dgutson
Copy link

dgutson commented Jul 20, 2023

I observed this change in @c4p-n1ck's fork:

master...c4p-n1ck:montydb:master#diff-822d717926e4514a846fad4aecc14e0cfcd9f30b60eca800667359921e6a5022R96

-       self.__conn = sqlite3.connect(db_file)
+      self.__conn = sqlite3.connect(db_file, check_same_thread=False)

Should that be brought here?

@davidlatwe
Copy link
Owner

Hmmm, just had a quick look.

I think what we could do is adding that check_same_thread as an config option at here:

def config(cls, journal_mode="WAL", **kwargs):

Then the config should be used to initialize a SQLite storage engine instance here:

class SQLiteKVEngine(object):
def __init__(self, db_pragmas):
self.__db_pragmas = db_pragmas
self.__conn = None

At the moment, SQLite config is being used to keep database pargmas, but it could do more than that. And check_same_thread is a good fit.

@dgutson
Copy link
Author

dgutson commented Jul 21, 2023

so it is not a race condition fix? Sorry about my ignorance.

@davidlatwe
Copy link
Owner

Don't be sorry! I am not familiar with this either.

But we certainly can add check_same_thread as a connection option though. Since it will affect the behavior of how a sqlite connection reacts to different thread, so we would want it to be optionable. ☺️

@davidlatwe
Copy link
Owner

Feature added in 2.5.2, closing this now. 😃

@dgutson
Copy link
Author

dgutson commented Jul 22, 2023

u rock!

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

2 participants