Intermittent parse() KeyError failure with 2.8.0

[kainjow]

We are seeing a KeyError exception with 2.8.0 intermittently. We have reverted to 2.7.5 for now. Seems like a thread safety issue with the new strong cache? Here's the traceback:

  File "site-packages\dateutil\parser\_parser.py", line 1358, in parse
  File "site-packages\dateutil\parser\_parser.py", line 657, in parse
  File "site-packages\dateutil\parser\_parser.py", line 1192, in _build_tzaware
  File "site-packages\dateutil\tz\_factories.py", line 40, in __call__
  File "collections.py", line 157, in pop
  File "collections.py", line 86, in __delitem__
KeyError: (None, -25200)

[pganssle]

Hmmm, very interesting. I'm not quite sure which operation is not thread-safe, but from the stack trace I'm guessing it's pop - looks like maybe two threads find that key is in the dictionary simultaneously, one thread retrieves the value and deletes it, the other one attempts to delete it but finds it's already been deleted. I wonder if this is a bug in OrderedDict.

In any case, I think we can fix it by switching this into two operations:

cls.__strong_cache.setdefault(key, instance)
cls.__strong_cache.move_to_end(key, last=True)

@kainjow Any way you might be able to put together a minimal reproducing script so that we can test that it's fixed?

[pganssle]

@kainjow By the way, what version of Python were you using? If this was a bug in OrderedDict that has been fixed, that would be good to know.

Looks like it might be this bug in CPython. Not sure which versions are affected.

[kainjow]

We're on 2.7.13 (upgrade to 3.7 is in progress).

[pganssle]

OK, so I have written a little reproducer, it seems that pop is not thread-safe in Python 2.7, but it seems to be thread-safe in all of Python 3:

import threading

from collections import OrderedDict
import sys

if sys.version_info[0] < 3:
    from threading import Thread,Semaphore

    class Barrier:
        def __init__(self, n):
            self.n = n
            self.count = 0
            self.mutex = Semaphore(1)
            self.barrier = Semaphore(0)

        def wait(self):
            self.mutex.acquire()
            self.count = self.count + 1
            self.mutex.release()
            if self.count == self.n: self.barrier.release()
            self.barrier.acquire()
            self.barrier.release()
else:
    from threading import Barrier

d = OrderedDict(a=4)
N = 500
b = Barrier(N)
b2 = Barrier(N+1)

def popper():
    b.wait()
    d['a'] = d.pop('a', 4)
    b2.wait()

for i in range(N):
    threading.Thread(target=popper).start()

b2.wait()
print("Done")

Maybe we can get the fix implemented in Python 2.7. In the meantime, while we still support Python 2, it might be a good idea to switch to the implementation in my earlier response, ideally with a note that we can switch it back when we drop Python 2.

[maccinza]

I am having this same issue in Python 2.7 and dateutil 2.7.5.
Any suggestions on how to implement a workaround for this on Python 2?

[piotrek204]

Hi,
I have the same problem. How about (code below)? I saw a similar solution elsewhere with __strong_cache.

class _TzOffsetFactory(_TzFactory):
    def __init__(cls, *args, **kwargs):
        cls.__instances = weakref.WeakValueDictionary()
        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())
        else:
            key = (name, offset)

        instance = cls.__instances.get(key, None)
        if instance is None:
            instance = cls.__instances.setdefault(key,
                                                  cls.instance(name, offset))

        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)

        return instance

[jacobg]

This rare parse() KeyError occurred for me running dateutil 2.8.1 on Python 2.7 (Google App Engine). It doesn't seem like this bug is fixed.