From 6e92f2111589ab061b66bd332dcb4a33a58acb84 Mon Sep 17 00:00:00 2001 From: Paul Ganssle Date: Sat, 2 Nov 2019 21:24:29 -0400 Subject: [PATCH] Lock around strong cache updates Since the strong cache uses operations in OrderedDict that are not thread-safe in Python 2, it is necessary to acquire a lock while updating the dictionary. When Python 2 support is dropped, we can likely refactor this to avoid locking. --- changelog.d/973.bugfix.rst | 1 + dateutil/tz/_factories.py | 29 ++++++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 changelog.d/973.bugfix.rst diff --git a/changelog.d/973.bugfix.rst b/changelog.d/973.bugfix.rst new file mode 100644 index 000000000..6af867c93 --- /dev/null +++ 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 index d2560eb73..f8a65891a 100644 --- 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