[jarun#753] implement thread safety #760
Conversation
| self.cur.execute(query, args) | ||
| return [BookmarkVar(*x) for x in self.cur.fetchall()] | ||
| with self.lock: | ||
| return self._fetch(query, *args, lock=False) |
There was a problem hiding this comment.
This change only affects concurrent access (which would throw an error before the fix)
| self.cur.execute(query, list(indices)) | ||
| resultset = self.cur.fetchall() | ||
| if not resultset: | ||
| return False |
There was a problem hiding this comment.
Moved the short branch from below
| @@ -909,8 +943,6 @@ class BukuDb: | |||
|
|
|||
| if not delay_commit: | |||
| self.conn.commit() | |||
| else: | |||
| return False | |||
There was a problem hiding this comment.
Moved the branch to above
| @@ -1231,146 +1265,139 @@ class BukuDb: | |||
| if not MYHEADERS: | |||
| gen_headers() | |||
|
|
|||
| cond = threading.Condition() | |||
| cond.acquire() | |||
There was a problem hiding this comment.
Moved this below
| count : int | ||
| Dummy input to adhere to convention. | ||
| thread_idx : int | ||
| Thread index/ID. |
There was a problem hiding this comment.
This can be used for logging etc.
| cond : threading condition object. | ||
| """ | ||
|
|
||
| count = 0 | ||
| _count = 0 |
There was a problem hiding this comment.
Ensuring it's a local variable
| if resultset: | ||
| id, url, tags, flags = resultset.pop() | ||
| else: | ||
| break |
There was a problem hiding this comment.
Not actually changing any behaviour here, but using with does away with explicit .release()
| @@ -1,4 +1,6 @@ | |||
| [pytest] | |||
| timeout = 10 | |||
| timeout_method = thread | |||
There was a problem hiding this comment.
Ensuring the tests won't freeze in case of a deadlock 😅
Note that default timeout-method is signal, but it doesn't appear to handle deadlocks well.
|
Thank you! |
…So I've been working on Bootstrap v4 migration (…as much as my free time permitted), and encountered a bug that caused every new modal to send twice as many fetch requests as the previous one when being opened. This exposed a problem in the main Buku library: it's not threadsafe. (More specifically, dealing with two simultaneous web-requests causes an error, and having more than that results in an immediate segfault.)
Hence, I've added a repeating-lock to handle concurrent access (it doesn't interfere with same-thread actions, thus only affecting the cases when the library is used from multiple threads at once), and published it as a separate pull-request.
I've made sure it doesn't cause issues with concurrent access, but just in case I've also added a 10 seconds time limit on execution of each test (to handle possible deadlocks).