Skip to content

Commit

Permalink
Lock around strong cache updates
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pganssle committed Nov 3, 2019
1 parent 2e36ff7 commit 6e92f21
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
1 change: 1 addition & 0 deletions 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).
29 changes: 18 additions & 11 deletions dateutil/tz/_factories.py
Expand Up @@ -2,6 +2,8 @@
import weakref
from collections import OrderedDict

from six.moves import _thread


class _TzSingleton(type):
def __init__(cls, *args, **kwargs):
Expand All @@ -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())
Expand All @@ -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

Expand All @@ -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)
Expand All @@ -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

0 comments on commit 6e92f21

Please sign in to comment.