Re: [openstack-dev] [all] i need some help on this bug Bug #1365892
On Wed, 10 Sep 2014 01:04:02 PM Mike Bayer wrote: > In this case it appears to be a “safety” in case someone uses the > ConnectionContext object outside of being a context manager. I’d fix that > and require that it be used as a context manager only. Oh look, I have a new pylint hammer that is designed for exactly this nail: https://review.openstack.org/#/c/120320/ -- - Gus ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] i need some help on this bug Bug #1365892
On Sep 10, 2014, at 4:11 AM, Li Tianqing wrote: > After some research, i find the reason for the cycle reference. In closure, > the _fix_paswords.func_closre reference the _fix_passwords. So the > cycle reference happened. > And in https://thp.io/2012/python-gc/python_gc_final_2012-01-22.pdf page 5, > it says that > We observe that Python implementations with distinct GCs behave differently: > CPython does not even try to get the order of finalizers right, and > simply puts uncollectable objects into the global list of garbage for the > developer to deal with manually. > So the gc can not handle cycle reference, then the memory leak happened. An object is only uncollectable in Python if it has a __del__ method and is part of an unreachable cycle. Based on a grep, there is only one class in oslo.messaging that has a __del__ and that is ConnectionContext in _drivers/amqp.py. Removing the __del__ method from this object would be the best approach. There’s nothing wrong with reference cycles in Python as long as we don’t use __del__, which IMHO has no legitimate use case. Additionally, it is very difficult to be 100% vigilant against reference cycles reappearing in many cases, but it is extremely simple to be 100% vigilant about disallowing __del__. In this case it appears to be a “safety” in case someone uses the ConnectionContext object outside of being a context manager. I’d fix that and require that it be used as a context manager only. Guessing that the user is going to mis-use an object and provide __del__ to protect the user is a bad idea - and if you genuinely need this pattern, you use a weakref callback instead: import weakref class MyCleanupThing(object): def __init__(self): self.connection = connection = "Some Connection" self._ref = weakref.ref( # key: the weakref callback *cannot* refer to self self, lambda ref: MyCleanupThing._cleanup(connection)) @staticmethod def _cleanup(connection): print("Cleaning up %s!" % connection) mc = MyCleanupThing() print("about to clean up...") del mc print("should be clean!") output: about to clean up... Cleaning up Some Connection! should be clean! > > If there is something wrong, please fix it. Thanks > > -- > Best > Li Tianqing > > 在 2014-09-10 11:52:28,"Li Tianqing" 写道: > Hello, > I use backdoor of eventlet to enable gc.DEBUG_LEAK, and after wait a few > minutes, i can sure that there will some objects that can not be collected by > gc.collect in gc.garbage. > Those looks like this (catched in ceilometer-collector) > > ['_context_auth_token', 'auth_token', 'new_pass'], > (, > ), > , > , > , > ['_context_auth_token', 'auth_token', 'new_pass'], > (, > ), > , > , > , > ['_context_auth_token', 'auth_token', 'new_pass'], > (, > ), > , > , > , > > and i suspect those code in oslo.messaging > > def _safe_log(log_func, msg, msg_data): > """Sanitizes the msg_data field before logging.""" > SANITIZE = ['_context_auth_token', 'auth_token', 'new_pass'] > > def _fix_passwords(d): > """Sanitizes the password fields in the dictionary.""" > for k in d.iterkeys(): > if k.lower().find('password') != -1: > d[k] = '' > elif k.lower() in SANITIZE: > d[k] = '' > elif isinstance(d[k], dict): > _fix_passwords(d[k]) > return d > > return log_func(msg, _fix_passwords(copy.deepcopy(msg_data))) > > i can resolve this problem by add _fix_passwords = None before _safe_log > returns. > > But i do not really understand why this problem happened, and in depth why > the gc can not collect those object. Although i can make those uncollectable > objects disappeared. > But this is not good enough, because if you do not understand it you will > write out some code like this in future, and then also has memory leak too. > > So can some one helps me give some detailed on recursive closure used like > the code above, and on why gc can not collect them. > Thanks a lot lot .. > > -- > Best > Li Tianqing > > > > > ___ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [all] i need some help on this bug Bug #1365892
After some research, i find the reason for the cycle reference. In closure, the _fix_paswords.func_closre reference the _fix_passwords. So the cycle reference happened. And in https://thp.io/2012/python-gc/python_gc_final_2012-01-22.pdf page 5, it says that We observe that Python implementations with distinct GCs behave differently: CPython does not even try to get the order of finalizers right, and simply puts uncollectable objects into the global list of garbage for the developer to deal with manually. So the gc can not handle cycle reference, then the memory leak happened. If there is something wrong, please fix it. Thanks -- Best Li Tianqing 在 2014-09-10 11:52:28,"Li Tianqing" 写道: Hello, I use backdoor of eventlet to enable gc.DEBUG_LEAK, and after wait a few minutes, i can sure that there will some objects that can not be collected by gc.collect in gc.garbage. Those looks like this (catched in ceilometer-collector) ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , and i suspect those code in oslo.messaging def _safe_log(log_func, msg, msg_data): """Sanitizes the msg_data field before logging.""" SANITIZE = ['_context_auth_token', 'auth_token', 'new_pass'] def _fix_passwords(d): """Sanitizes the password fields in the dictionary.""" for k in d.iterkeys(): if k.lower().find('password') != -1: d[k] = '' elif k.lower() in SANITIZE: d[k] = '' elif isinstance(d[k], dict): _fix_passwords(d[k]) return d return log_func(msg, _fix_passwords(copy.deepcopy(msg_data))) i can resolve this problem by add _fix_passwords = None before _safe_log returns. But i do not really understand why this problem happened, and in depth why the gc can not collect those object. Although i can make those uncollectable objects disappeared. But this is not good enough, because if you do not understand it you will write out some code like this in future, and then also has memory leak too. So can some one helps me give some detailed on recursive closure used like the code above, and on why gc can not collect them. Thanks a lot lot .. -- Best Li Tianqing ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [all] i need some help on this bug Bug #1365892
Hello, I use backdoor of eventlet to enable gc.DEBUG_LEAK, and after wait a few minutes, i can sure that there will some objects that can not be collected by gc.collect in gc.garbage. Those looks like this (catched in ceilometer-collector) ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , ['_context_auth_token', 'auth_token', 'new_pass'], (, ), , , , and i suspect those code in oslo.messaging def _safe_log(log_func, msg, msg_data): """Sanitizes the msg_data field before logging.""" SANITIZE = ['_context_auth_token', 'auth_token', 'new_pass'] def _fix_passwords(d): """Sanitizes the password fields in the dictionary.""" for k in d.iterkeys(): if k.lower().find('password') != -1: d[k] = '' elif k.lower() in SANITIZE: d[k] = '' elif isinstance(d[k], dict): _fix_passwords(d[k]) return d return log_func(msg, _fix_passwords(copy.deepcopy(msg_data))) i can resolve this problem by add _fix_passwords = None before _safe_log returns. But i do not really understand why this problem happened, and in depth why the gc can not collect those object. Although i can make those uncollectable objects disappeared. But this is not good enough, because if you do not understand it you will write out some code like this in future, and then also has memory leak too. So can some one helps me give some detailed on recursive closure used like the code above, and on why gc can not collect them. Thanks a lot lot .. -- Best Li Tianqing___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev