facetoe / zenpy

Python wrapper for the Zendesk API
GNU General Public License v3.0
340 stars 162 forks source link

Zenpy Cachetools implementation is not ThreadSafe and doesn't have a lock #610

Closed Vygo closed 7 months ago

Vygo commented 9 months ago

We've been getting KeyError from Cachetool and after internal investigation we found out that it is coming from with the zenpy usage of cachetools

Zenpy uses cachetools internally and doesn’t have a lock

In zenpy/lib/cache there exists class ZenpyCache(object): which has a lock around the purge function but nothing else.

def purge(self):
    """ Purge the cache of all items. """
    with self.purge_lock:
        self.cache.clear()

The issue is that other functions on the class such as

def __delitem__(self, key):
    del self.cache[key]

have no lock, and if multiple deletes happen at the same time in different threads, it causes a key error. This is the sequence of events within cachetools

Thread 1: Check for things ready for delete -> Finds key ABC123
Thread 2: Check for things ready for delete -> Finds key ABC123
Thread 1: Deletes ABC123 -> Success
Thread 2: Deletes ABC123 -> KeyError!
cryptomail commented 9 months ago

Interesting.....I don't know the state of the project as it relates to thread safety but this potentially could be shored up. Would you like to:

  1. create a PR against this to synchronize access/deletes?
  2. create tests that prove you've defeated the race conditions?
cryptomail commented 7 months ago

Closing due to inactivity. While I think it's a good idea, no one has stepped up to take this issue on.