• 
      

    Fix a memory leak with the new in-database SVN cert storage.

    Review Request #3494 — Created Nov. 12, 2012 and submitted

    Information

    Review Board
    release-1.6.x

    Reviewers

    Fix a memory leak with the new in-database SVN cert storage.
    
    The new SVN cert support introduced a memory leak due to a circular
    dependency.
    
    The client was holding on to a reference to the function on SVNTool,
    while SVNTool was holding on to the client, preventing either from being
    cleaned up.
    
    An attempt was made at inlining the function, but as that accessed data
    bound to the class, the circular dependency remained.
    
    This appears to be an issue specifically with PySVN, as an independent
    repro case could be devised for pysvn.Client's callbacks, but not those
    from a mock object.
    
    The callback is now defined as a classmethod and the only attribute it
    needs, a repository, is set up as a weakptr which is then accessed by a
    lambda calling the callback. This prevents the repository and SVNTool
    from remaining around indefinitely.
    Created a repro case that demonstrated the behavior:
    
    ####
    import pysvn
    
    
    class A(object):
        pass
    
    class MyClient(object):
        def __init__(self):
            self.b = A()
            self.client = pysvn.Client()
            self.client.callback_ssl_server_trust_prompt = self.my_prompt
    
        def my_prompt(self, trust_dict):
            return self.b, trust_dict, False
    
    for i in xrange(1, 1000000):
        client = MyClient()
    ####
    
    
    And then verified the fix with:
    
    
    ####
    import pysvn
    import weakref
    
    
    class A(object):
        pass
    
    class MyClient(object):
        def __init__(self):
            self.b = A()
            ref = weakref.ref(self.b)
    
            self.client = pysvn.Client()
            self.client.callback_ssl_server_trust_prompt = \
                lambda x: MyClient.my_prompt(x, ref())
    
        @classmethod
        def my_prompt(cls, trust_dict, b):
            return b, trust_dict, False
    
    for i in xrange(1, 1000000):
        client = MyClient()
    ####
    
    
    Also tested variants with and without a lambda.
    
    Unit tests pass.
    david
    1. Ship It!
    2. 
        
    chipx86
    david
    1. The code looks good. In your description, you say "weakptr" where I think you mean "weakref"
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-1.6.x (9f316c9)