Fix a bug with caching tiles and styling.

If multiple threads are styling tiles, it was possible for the tile
cache key to be erroneous due to the styler fetching the "unstyled" tile
and a second thread fetching the tile cache reference based on the name
of the unstyled class rather than the styled class.

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bug Fixes
 - Allow clearing the min/max fields of the frame selector ([#1030](../../pull/1030))
+- Fix a bug with caching tiles and styling ([#1031](../../pull/1031))
 
 ## 1.20.5
 

girder/girder_large_image/models/image_item.py

@@ -190,10 +190,12 @@ def _tileFromHash(cls, item, x, y, z, mayRedirect=False, **kwargs):
             sourceClass = girder_tilesource.AvailableGirderTileSources[sourceName]
         except TileSourceError:
             return None
-        if '_' in kwargs:
+        if '_' in kwargs or 'style' in kwargs:
             kwargs = kwargs.copy()
             kwargs.pop('_', None)
         classHash = sourceClass.getLRUHash(item, **kwargs)
+        # style isn't part of the tile hash strhash parameters
+        kwargs.pop('style', None)
         tileHash = sourceClass.__name__ + ' ' + classHash + ' ' + strhash(
             sourceClass.__name__ + ' ' + classHash) + strhash(
             *(x, y, z), mayRedirect=mayRedirect, **kwargs)

large_image/cache_util/cache.py

@@ -59,7 +59,7 @@ def strhash(*args, **kwargs):
     return '%r' % (args, )
 
 
-def methodcache(key=None):
+def methodcache(key=None):  # noqa
     """
     Decorator to wrap a function with a memoizing callable that saves results
     in self.cache.  This is largely taken from cachetools, but uses a cache
@@ -72,9 +72,17 @@ def decorator(func):
         @functools.wraps(func)
         def wrapper(self, *args, **kwargs):
             k = key(*args, **kwargs) if key else self.wrapKey(*args, **kwargs)
-            if hasattr(self, '_classkey'):
-                k = self._classkey + ' ' + k
             lock = getattr(self, 'cache_lock', None)
+            ck = getattr(self, '_classkey', None)
+            if lock:
+                with self.cache_lock:
+                    if hasattr(self, '_classkeyLock'):
+                        if self._classkeyLock.acquire(blocking=False):
+                            self._classkeyLock.release()
+                        else:
+                            ck = getattr(self, '_unlocked_classkey', ck)
+            if ck:
+                k = ck + ' ' + k
             try:
                 if lock:
                     with self.cache_lock:

large_image/tilesource/base.py

@@ -178,6 +178,11 @@ def _setStyle(self, style):
 
         :param style: The new style.
         """
+        for key in {'_unlocked_classkey', '_classkeyLock'}:
+            try:
+                delattr(self, key)
+            except Exception:
+                pass
         self._bandRanges = {}
         self._jsonstyle = style
         if style:
@@ -1042,10 +1047,7 @@ def _scanForMinMax(self, dtype, frame=None, analysisSize=1024, onlyMinMax=True,
         :param onlyMinMax: if True, only find the min and max.  If False, get
             the entire histogram.
         """
-        self._skipStyle = True
-        # Divert the tile cache while querying unstyled tiles
-        classkey = self._classkey
-        self._classkey = self._unstyledClassKey()
+        self._setSkipStyle(True)
         try:
             self._bandRanges[frame] = self.histogram(
                 dtype=dtype,
@@ -1059,8 +1061,7 @@ def _scanForMinMax(self, dtype, frame=None, analysisSize=1024, onlyMinMax=True,
                     k: v for k, v in self._bandRanges[frame].items() if k in {
                         'min', 'max', 'mean', 'stdev'}})
         finally:
-            del self._skipStyle
-            self._classkey = classkey
+            self._setSkipStyle(False)
 
     def _validateMinMaxValue(self, value, frame, dtype):
         """
@@ -1306,6 +1307,19 @@ def _applyICCProfile(self, sc, frame):
                 self.logger.exception('Failed to apply ICC profile')
         return sc.iccimage
 
+    def _setSkipStyle(self, setSkip=False):
+        if setSkip:
+            self._unlocked_classkey = self._classkey
+            if hasattr(self, 'cache_lock'):
+                with self.cache_lock:
+                    self._classkeyLock = self._styleLock
+            self._skipStyle = True
+            # Divert the tile cache while querying unstyled tiles
+            self._classkey = self._unstyledClassKey()
+        else:
+            del self._skipStyle
+            self._classkey = self._unlocked_classkey
+
     def _applyStyle(self, image, style, x, y, z, frame=None):  # noqa
         """
         Apply a style to a numpy image.
@@ -1349,18 +1363,14 @@ def _applyStyle(self, image, style, x, y, z, frame=None):  # noqa
             else:
                 frame = entry['frame'] if entry.get('frame') is not None else (
                     sc.mainFrame + entry['framedelta'])
-                self._skipStyle = True
-                # Divert the tile cache while querying unstyled tiles
-                classkey = self._classkey
-                self._classkey = self._unstyledClassKey()
+                self._setSkipStyle(True)
                 try:
                     image = self.getTile(x, y, z, frame=frame, numpyAllowed=True)
                     image = image[:sc.mainImage.shape[0],
                                   :sc.mainImage.shape[1],
                                   :sc.mainImage.shape[2]]
                 finally:
-                    del self._skipStyle
-                    self._classkey = classkey
+                    self._setSkipStyle(False)
             if (isinstance(entry.get('band'), int) and
                     entry['band'] >= 1 and entry['band'] <= image.shape[2]):
                 sc.bandidx = entry['band'] - 1