From 6d8e0a82f6e6bb6a5d7833fa431b750c1b17b644 Mon Sep 17 00:00:00 2001 From: Ramana Raja Date: Thu, 2 Nov 2023 17:44:10 -0400 Subject: [PATCH] pybind/mgr: remove __del__() of mgr modules It's strongly recommended for objects that have references to external resources (e.g., files) to explicitly release them. Python doesn't guarantee garbage collection of objects and hence doesn't guarantee freeing of external resources that occur on garbage collection. The __del__() methods in the python mgr modules may not even be called since garbage collection of objects is not guaranteed in python. And some of the __del__() methods try to cleanup that seem redundant. - In volumes/module.py, vc.shutdown() is called in Module.shutdown(). No need to call it again in Module.__del__() - In telegraf/basesocket.py, BaseSocker.close() is called in BaseSocket.__exit__(). No need to call it again in BaseSocket.__del__(). - In mgr_module.py, MgrModuleLoggingMixin._unconfigure_logging() is called in MgrModule.__init__() and MgrStandbyModule.__init__(). No need to call it in MgrModule.__del__() and MgrStandbyModule.__del__().| - In dashboard/services/cephfs.py, the libcephfs mount is not shutdown explicitly by the mgr module. However, the cython libcephfs bindings has a LibCephFS.__dealloc__() finalizer method that calls LibCephFS.shutdown(). This should unmount and cleanup the ceph mount handle. Remove the __del__() of the python mgr modules. Fixes: https://tracker.ceph.com/issues/63421 Signed-off-by: Ramana Raja --- src/pybind/mgr/dashboard/services/cephfs.py | 4 ---- src/pybind/mgr/mgr_module.py | 10 ---------- src/pybind/mgr/telegraf/basesocket.py | 3 --- src/pybind/mgr/volumes/module.py | 3 --- 4 files changed, 20 deletions(-) diff --git a/src/pybind/mgr/dashboard/services/cephfs.py b/src/pybind/mgr/dashboard/services/cephfs.py index 07b339cc92176..ffbf9d0c81651 100644 --- a/src/pybind/mgr/dashboard/services/cephfs.py +++ b/src/pybind/mgr/dashboard/services/cephfs.py @@ -45,10 +45,6 @@ class CephFS(object): self.cfs.mount() logger.debug("mounted cephfs filesystem") - def __del__(self): - logger.debug("shutting down cephfs filesystem") - self.cfs.shutdown() - @contextmanager def opendir(self, dirpath): d = None diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 5a7b9bfc6f6c8..6c83f2771619f 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -836,13 +836,6 @@ class MgrStandbyModule(ceph_module.BaseMgrStandbyModule, MgrModuleLoggingMixin): # for backwards compatibility self._logger = self.getLogger() - def __del__(self) -> None: - self._cleanup() - self._unconfigure_logging() - - def _cleanup(self) -> None: - pass - @classmethod def _register_options(cls, module_name: str) -> None: cls.MODULE_OPTIONS.append( @@ -1045,9 +1038,6 @@ class MgrModule(ceph_module.BaseMgrModule, MgrModuleLoggingMixin): self._db_lock = threading.Lock() - def __del__(self) -> None: - self._unconfigure_logging() - @classmethod def _register_options(cls, module_name: str) -> None: cls.MODULE_OPTIONS.append( diff --git a/src/pybind/mgr/telegraf/basesocket.py b/src/pybind/mgr/telegraf/basesocket.py index 5caea3be72596..463cf326dd057 100644 --- a/src/pybind/mgr/telegraf/basesocket.py +++ b/src/pybind/mgr/telegraf/basesocket.py @@ -38,9 +38,6 @@ class BaseSocket(object): def send(self, data: str, flags: int = 0) -> int: return self.sock.send(data.encode('utf-8') + b'\n', flags) - def __del__(self) -> None: - self.sock.close() - def __enter__(self) -> 'BaseSocket': self.connect() return self diff --git a/src/pybind/mgr/volumes/module.py b/src/pybind/mgr/volumes/module.py index ff7256eebfd34..6227276fcaf57 100644 --- a/src/pybind/mgr/volumes/module.py +++ b/src/pybind/mgr/volumes/module.py @@ -506,9 +506,6 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): self.vc = VolumeClient(self) self.inited = True - def __del__(self): - self.vc.shutdown() - def shutdown(self): self.vc.shutdown() -- 2.39.5