Ensure that updating the vl_book cache is both thread safe and recursion safe.

This commit is contained in:
Charles Haley 2021-05-05 18:59:45 +01:00
parent 0a6010e712
commit 312e44dc3b

View File

@ -15,6 +15,7 @@ import traceback
from collections import MutableSet, Set, defaultdict from collections import MutableSet, Set, defaultdict
from functools import partial, wraps from functools import partial, wraps
from io import BytesIO from io import BytesIO
from threading import Lock
from time import time from time import time
from calibre import as_unicode, isbytestring from calibre import as_unicode, isbytestring
@ -142,7 +143,7 @@ class Cache(object):
self.formatter_template_cache = {} self.formatter_template_cache = {}
self.dirtied_cache = {} self.dirtied_cache = {}
self.vls_for_books_cache = None self.vls_for_books_cache = None
self.vls_for_books_cache_is_loading = False self.vls_cache_lock = Lock()
self.dirtied_sequence = 0 self.dirtied_sequence = 0
self.cover_caches = set() self.cover_caches = set()
self.clear_search_cache_count = 0 self.clear_search_cache_count = 0
@ -252,7 +253,6 @@ class Cache(object):
self.clear_search_cache_count += 1 self.clear_search_cache_count += 1
self._search_api.update_or_clear(self, book_ids) self._search_api.update_or_clear(self, book_ids)
self.vls_for_books_cache = None self.vls_for_books_cache = None
self.vls_for_books_cache_is_loading = False
@read_api @read_api
def last_modified(self): def last_modified(self):
@ -2232,26 +2232,35 @@ class Cache(object):
if self.vls_for_books_cache is None: if self.vls_for_books_cache is None:
# Using a list is slightly faster than a set. # Using a list is slightly faster than a set.
c = defaultdict(list) c = defaultdict(list)
if self.vls_for_books_cache_is_loading: got_lock = False
# We get here if resolving the books in a VL triggers another VL try:
# calculation. This can be 'real' recursion, in which case the # use a primitive lock to ensure that only one thread is updating
# eventual answer will be wrong. It can also be a search using # the cache and that recursive calls don't do the update. This
# a location of 'all' that causes evaluation of a composite that # method can recurse via self._search()
# references virtual libraries. If the composite isn't used in a got_lock = self.vls_cache_lock.acquire(blocking=False)
# VL then the eventual answer will be correct because get_metadata if not got_lock:
# will clear the caches. # We get here if resolving the books in a VL triggers another VL
return c # calculation. This can be 'real' recursion, in which case the
self.vls_for_books_cache_is_loading = True # eventual answer will be wrong. It can also be a search using
libraries = self._pref('virtual_libraries', {}) # a location of 'all' that causes evaluation of a composite that
for lib, expr in libraries.items(): # references virtual_libraries(). If the composite isn't used in a
book = None # VL then the eventual answer will be correct because get_metadata
try: # will clear the caches.
for book in self._search(expr, virtual_fields=virtual_fields): return c
c[book].append(lib) self.vls_for_books_cache_is_loading = True
except Exception as e: libraries = self._pref('virtual_libraries', {})
if book: for lib, expr in libraries.items():
c[book].append(_('[Error in Virtual library {0}: {1}]').format(lib, str(e))) book = None
self.vls_for_books_cache = {b:tuple(sorted(libs, key=sort_key)) for b, libs in c.items()} try:
for book in self._search(expr, virtual_fields=virtual_fields):
c[book].append(lib)
except Exception as e:
if book:
c[book].append(_('[Error in Virtual library {0}: {1}]').format(lib, str(e)))
self.vls_for_books_cache = {b:tuple(sorted(libs, key=sort_key)) for b, libs in c.items()}
finally:
if got_lock:
self.vls_cache_lock.release()
if not book_ids: if not book_ids:
book_ids = self._all_book_ids() book_ids = self._all_book_ids()
# book_ids is usually 1 long. The loop will be faster than a comprehension # book_ids is usually 1 long. The loop will be faster than a comprehension