Merge pull request #973 from pganssle/strong_cache_race

pganssle · web-flow · commit 357c62cf9b9e · 2019-11-02T21:45:29.000-04:00
Lock around strong cache updates
diff --git a/changelog.d/973.bugfix.rst b/changelog.d/973.bugfix.rst
@@ -0,0 +1 @@
+Fixed a race condition in the ``tzoffset`` and ``tzstr`` "strong" caches on Python 2.7. Reported by @kainjow (gh issue #901).
diff --git a/dateutil/tz/_factories.py b/dateutil/tz/_factories.py
@@ -2,6 +2,8 @@
 import weakref
 from collections import OrderedDict
 
+from six.moves import _thread
+
 
 class _TzSingleton(type):
     def __init__(cls, *args, **kwargs):
@@ -26,6 +28,8 @@ def __init__(cls, *args, **kwargs):
         cls.__strong_cache = OrderedDict()
         cls.__strong_cache_size = 8
 
+        cls._cache_lock = _thread.allocate_lock()
+
     def __call__(cls, name, offset):
         if isinstance(offset, timedelta):
             key = (name, offset.total_seconds())
@@ -37,12 +41,13 @@ def __call__(cls, name, offset):
             instance = cls.__instances.setdefault(key,
                                                   cls.instance(name, offset))
 
-        cls.__strong_cache[key] = cls.__strong_cache.pop(key, instance)
+        # This lock may not be necessary in Python 3. See GH issue #901
+        with cls._cache_lock:
+            cls.__strong_cache[key] = cls.__strong_cache.pop(key, instance)
 
-        # Remove an item if the strong cache is overpopulated
-        # TODO: Maybe this should be under a lock?
-        if len(cls.__strong_cache) > cls.__strong_cache_size:
-            cls.__strong_cache.popitem(last=False)
+            # Remove an item if the strong cache is overpopulated
+            if len(cls.__strong_cache) > cls.__strong_cache_size:
+                cls.__strong_cache.popitem(last=False)
 
         return instance
 
@@ -53,6 +58,8 @@ def __init__(cls, *args, **kwargs):
         cls.__strong_cache = OrderedDict()
         cls.__strong_cache_size = 8
 
+        cls.__cache_lock = _thread.allocate_lock()
+
     def __call__(cls, s, posix_offset=False):
         key = (s, posix_offset)
         instance = cls.__instances.get(key, None)
@@ -61,13 +68,13 @@ def __call__(cls, s, posix_offset=False):
             instance = cls.__instances.setdefault(key,
                 cls.instance(s, posix_offset))
 
-        cls.__strong_cache[key] = cls.__strong_cache.pop(key, instance)
-
+        # This lock may not be necessary in Python 3. See GH issue #901
+        with cls.__cache_lock:
+            cls.__strong_cache[key] = cls.__strong_cache.pop(key, instance)
 
-        # Remove an item if the strong cache is overpopulated
-        # TODO: Maybe this should be under a lock?
-        if len(cls.__strong_cache) > cls.__strong_cache_size:
-            cls.__strong_cache.popitem(last=False)
+            # Remove an item if the strong cache is overpopulated
+            if len(cls.__strong_cache) > cls.__strong_cache_size:
+                cls.__strong_cache.popitem(last=False)
 
         return instance
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fixed a race condition in the ``tzoffset`` and ``tzstr`` "strong" caches on Python 2.7. Reported by @kainjow (gh issue #901).