-
Notifications
You must be signed in to change notification settings - Fork 131
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
Fix deadlock where opening database fails #107
Conversation
…ion; avoid deadlock after race condition where command is enqueued before thread can signal an exception piskvorky#90
@mpenkov Any updates on getting this PR merged? I've tested this PR in our own fork and it pretty much fixes the issue, and has been stable so far (been using this patch for a couple of months now). Just though I'd ping you on this, as it would be useful patch for others to have |
sqlitedict.py
Outdated
conn = sqlite3.connect(self.filename, check_same_thread=False) | ||
except Exception as ex: | ||
self.log.exception("Failed to initialize connection for filename: %s" % self.filename) | ||
self.exception = (e_type, e_value, e_tb) = sys.exc_info() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of declaring the new variables?
self.exception = (e_type, e_value, e_tb) = sys.exc_info() | |
self.exception = sys.exc_info() |
sqlitedict.py
Outdated
@@ -552,6 +573,28 @@ def close(self, force=False): | |||
self.select_one('--close--') | |||
self.join() | |||
|
|||
def wait_for_initialization(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth making this thing explicitly internal, so people don't touch it (or at least expect bad things to happen if they do).
def wait_for_initialization(self): | |
def _wait_for_initialization(self): |
sqlitedict.py
Outdated
@@ -390,19 +395,34 @@ def __init__(self, filename, autocommit, journal_mode): | |||
self.reqs = Queue() | |||
self.setDaemon(True) # python2.5-compatible | |||
self.exception = None | |||
self.initialized = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.initialized = None | |
self._initialized = None |
Thank you for reminding me about this. Let's work on getting this merged. I left you some minor comments, please have a look. |
This isn't my PR, so I'll wait for the original author to jump in @padelt |
Hey everyone - nearly forget about this PR. |
Had to rename the variable again to avoid messing up @mpenkov Please have a look at the line Tests now pass: https://github.com/padelt/sqlitedict/runs/1757442319 |
Remove merge artifact
Merged. Thank you for your contribution and your patience @padelt ! |
Hello, are there any plans to release this in a new version to PyPI? It appears to address an issue that I'm having with the latest official release. Thanks! |
...without setting self.exception; avoid deadlock after race condition where command is enqueued before thread can signal an exception #90