Open Stack

On Sep 10, 2014, at 4:11 AM, Li Tianqing <jazeltq at 163.com> 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" <jazeltq at 163.com> 写道：
> 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'],
>  (<cell at 0x363bc58: list object at 0x361c170>,
>   <cell at 0x363bda8: function object at 0x361a5f0>),
>  <function _fix_passwords at 0x361a5f0>,
>  <cell at 0x363bde0: list object at 0x363c680>,
>  <cell at 0x363bd70: function object at 0x361a668>,
>  ['_context_auth_token', 'auth_token', 'new_pass'],
>  (<cell at 0x363bde0: list object at 0x363c680>,
>   <cell at 0x363bd70: function object at 0x361a668>),
>  <function _fix_passwords at 0x361a668>,
>  <cell at 0x363bf30: list object at 0x361b680>,
>  <cell at 0x363e168: function object at 0x361a758>,
>  ['_context_auth_token', 'auth_token', 'new_pass'],
>  (<cell at 0x363bf30: list object at 0x361b680>,
>   <cell at 0x363e168: function object at 0x361a758>),
>  <function _fix_passwords at 0x361a758>,
>  <cell at 0x363e1a0: list object at 0x363cdd0>,
>  <cell at 0x363e130: function object at 0x361a7d0>,
> 
> 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] = '<SANITIZED>'
>             elif k.lower() in SANITIZE:
>                 d[k] = '<SANITIZED>'
>             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 at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140910/a6e58056/attachment.html>