Cleanup #63421
Updated by Ramana Raja 7 months ago
From https://github.com/ceph/ceph/pull/54251#discussion_r1378165335, " What I don't understand there is why in https://github.com/ceph/ceph/pull/30890 #30890 it was deemed necessary to call `VolumeClient.shutdown()` from both `Module.shutdown()` and `Module.__del__()`. It doesn't make sense to me because: - `Module.__del__()` would end up doing shutdown work only if the last reference to `Module` somehow got put _without_ `Module.shutdown()` being called first. Otherwise there would be no shutdown work to do. - It's the manager framework who was expected to call `Module.shutdown()` from C++ via `PyModuleRunner::shutdown()`. - It's the (same) manager framework who is holding at least one reference to `Module`. So either I'm missing something, or all `__del__()` methods in modules (`MgrModule` subclasses) where something is closed or "shut down" are bogus. `__del__()` isn't even guaranteed to be called: https://docs.python.org/3.11/reference/datamodel.html > An implementation is allowed to postpone garbage collection or omit it altogether — it is a matter of implementation quality how garbage collection is implemented [...] > [...] Do not depend on immediate finalization of objects when they become unreachable (so you should always close files explicitly)." Also observed that in - mgr/mgr_module.py, MgrModule.__del__() and MgrStandbyModule.__del__() call `MgrModuleLoggingMixin.unconfigure_logging()`, which is also called in `MgrModuleLoggingMixin._configure_logging()` - mgr/telegraf/basesocket.py, BaseSocket.__del__() tries to close a socket. The socket is also closed by BaseSocker.__exit__()