Review Board 2.0 RC2 (dev)


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

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

Information
Christian Hammond
Review Board
release-1.6.x
Reviewers
reviewboard
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.
Review request changed
Updated (Nov. 19, 2012, 4:01 p.m.)

Status: Closed (submitted)

Change Summary:

Pushed to release-1.6.x (9f316c9)