From 43713c94ecdbffa06108797e70f235ba0aafc221 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Mon, 10 Jun 2024 22:28:58 +0200 Subject: [PATCH 1/2] fix: Replace __del__ with weakref.finalize in DictCache --- src/DIRAC/Core/Utilities/DictCache.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/DIRAC/Core/Utilities/DictCache.py b/src/DIRAC/Core/Utilities/DictCache.py index 734fe52f0e1..778fd0ffe97 100644 --- a/src/DIRAC/Core/Utilities/DictCache.py +++ b/src/DIRAC/Core/Utilities/DictCache.py @@ -3,6 +3,7 @@ """ import datetime import threading +import weakref # DIRAC from DIRAC.Core.Utilities.LockRing import LockRing @@ -64,6 +65,8 @@ def __init__(self, deleteFunction=False, threadLocal=False): # Function to clean the elements self.__deleteFunction = deleteFunction + self.__finalizer = weakref.finalize(self, self.purgeAll, useLock=False) + @property def lock(self): """Return the lock. @@ -236,17 +239,3 @@ def purgeAll(self, useLock=True): finally: if useLock: self.lock.release() - - def __del__(self): - """When the DictCache is deleted, all the entries should be purged. - This is particularly useful when the DictCache manages files - CAUTION: if you carefully read the python doc, you will see all the - caveat of __del__. In particular, no guaranty that it is called... - (https://docs.python.org/2/reference/datamodel.html#object.__del__) - """ - self.purgeAll(useLock=False) - del self.__lock - if self.__threadLocal: - del self.__threadLocalCache - else: - del self.__sharedCache From 49913a35f6510fc10eb8cb0433a1ab80ba0b1b1c Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Tue, 11 Jun 2024 21:42:19 +0200 Subject: [PATCH 2/2] fix: Break reference cycles around DictCache --- src/DIRAC/Core/Utilities/DictCache.py | 34 ++++++++++++------- .../Client/ProxyManagerClient.py | 3 +- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/DIRAC/Core/Utilities/DictCache.py b/src/DIRAC/Core/Utilities/DictCache.py index 778fd0ffe97..7ce5b1f5fec 100644 --- a/src/DIRAC/Core/Utilities/DictCache.py +++ b/src/DIRAC/Core/Utilities/DictCache.py @@ -65,7 +65,8 @@ def __init__(self, deleteFunction=False, threadLocal=False): # Function to clean the elements self.__deleteFunction = deleteFunction - self.__finalizer = weakref.finalize(self, self.purgeAll, useLock=False) + # Called when this object is deleted or the program ends + self.__finalizer = weakref.finalize(self, _purgeAll, None, self.__cache, self.__deleteFunction) @property def lock(self): @@ -225,17 +226,26 @@ def purgeExpired(self, expiredInSeconds=0): def purgeAll(self, useLock=True): """Purge all entries - CAUTION: useLock parameter should ALWAYS be True except when called from __del__ + CAUTION: useLock parameter should ALWAYS be True :param bool useLock: use lock """ - if useLock: - self.lock.acquire() - try: - for cKey in list(self.__cache): - if self.__deleteFunction: - self.__deleteFunction(self.__cache[cKey]["value"]) - del self.__cache[cKey] - finally: - if useLock: - self.lock.release() + _purgeAll(self.lock if useLock else None, self.__cache, self.__deleteFunction) + + +def _purgeAll(lock, cache, deleteFunction): + """Purge all entries + + This is split in to a helper function to be used by the finalizer without + needing to add a reference to the DictCache object itself. + """ + if lock: + lock.acquire() + try: + for cKey in list(cache): + if deleteFunction: + deleteFunction(cache[cKey]["value"]) + del cache[cKey] + finally: + if lock: + lock.release() diff --git a/src/DIRAC/FrameworkSystem/Client/ProxyManagerClient.py b/src/DIRAC/FrameworkSystem/Client/ProxyManagerClient.py index a32857efa17..4fb4089defa 100755 --- a/src/DIRAC/FrameworkSystem/Client/ProxyManagerClient.py +++ b/src/DIRAC/FrameworkSystem/Client/ProxyManagerClient.py @@ -30,7 +30,8 @@ def __init__(self): self.__pilotProxiesCache = DictCache() self.__filesCache = DictCache(self.__deleteTemporalFile) - def __deleteTemporalFile(self, filename): + @staticmethod + def __deleteTemporalFile(filename): """Delete temporal file :param str filename: path to file