Memory leak per request with add_request_method and resource_url

[murrayreadccdc]

I am finding a continuous memory growth in my Pyramid application when I make repeated requests to it. This is happening both with pserve on windows and apache on linux.

A test pserve process died due to memory exhaustion after 85 minutes with 1871MB allocated under a load of up to 40 requests per second.

I have managed to reproduce the issue in a minimal pyramid application, based on the standard hello world app. There are two key lines added to this, which cause the problem. First "config.add_request_method(db, property=True)" - if this is used with either property=True or reify=True, you get the leak, otherwise not. Second "url = request.resource_url(request.context)" - without this call to get a resource url, there is no leak. Here is the full test app:

from wsgiref.simple_server import make_server
from pyramid.config import Configurator
from pyramid.response import Response

def hello_world(request):
    url = request.resource_url(request.context)
    return Response('Hello world! ' + url)

def db(request):
    return None

if __name__ == '__main__':
    config = Configurator()
    config.add_route('hello', '/')
    config.add_view(hello_world, route_name='hello')
    config.add_request_method(db, property=True)
    app = config.make_wsgi_app()
    server = make_server('0.0.0.0', 8080, app)
    server.serve_forever()

I am using the following test client app to drive the server:

import urllib2
import sys
import time

uri = sys.argv[1]

count = 0
while True:
    f = urllib2.urlopen(uri)
    print count, len(f.read())
    f.close()
    count += 1
    time.sleep(0.01)

Monitoring the memory use in the pyramid app, it increases by 1MB every few seconds. This rate stays approximately constant.

[murrayreadccdc]

The test app memory on windows grows at a rate of 4388 bytes per request, average over 50K requests.

[wichert]

Running this with this gc tween checker shows the following python objects being created for each request:

ClassProvides                  1 +
Implements                     1 +
Response                       1 +
dict                           8 +
function                       2 +
instance                       3 +
list                           2 +
tuple                          10 +
weakref                        7 +

Removing the request method removes changes that to this:

Response                       1 +
dict                           1 +
instance                       1 +
list                           2 +

[wichert]

I traced this down to InstancePropertyMixin._set_properties. If I comment out the last line (self.__class__ = cls) the memory leak goes away.

[wichert]

One thing I am wondering here is why a new type is created for every request instance? Isn't it possible to create the type once when the WSGI app is created and then keep using that?

[mmerickel]

The class/type of the request object is dependent on the request factory used in the app. It's not predictable at configuration time.

[mmerickel]

The other way to implement the InstancePropertyMixin is to override the __getattribute__ magic method, but this is not ideal because it has a performance impact on every attribute lookup on the request object. Perhaps there is a way for us to break whatever circular reference is being created between the class objects at the end of the request?

[wichert]

When make_wsgi_app is called the configuration is complete. At that point you could create the right request type and set that on the Router instance.

[mmerickel]

Incorrect, the request type is completely dependent on the request factory the user supplies, which is a per-request factory of the format request = request_factory(environ).

[mmerickel]

This issue is related to #1067.

[wichert]

Hmm. I wonder what the use case of a per-request factory is. It feels like Zopish over-engineering, but I'm guessing Chris had a valid use case I can't think of.

[mmerickel]

It was the way to add custom properties to a request until we added request properties in 1.3. There are plenty of other reasons as well, for example replacing webob or custom request subclasses based on some headers. The type of the request is something that pyramid dispatches based on as well.

[wichert]

Before request properties I can see the point of providing a custom Request type, but not in having a different request type per HTTP request. I still can't think of any reason why you would need that.

[wichert]

The analysis from #1067 seems to match the extra objects I see, which suggests this may be a bug in the ZTK.

[mcdonc]

What's happening here is this:

• The new class is created and assigned as __class__ to the instance. In and of itself this does not cause any memory leak.
• However, later on, when the request instance is used as part of a queryMultiAdapter call, the zope.interface code eventually tries to look up the attribute in the class' __dict__ named __implemented__ (within zope.interface.declarations.implementedBy or zope.interface.declarations.implementedByFallback). Because the newly created class does not have a dict member named __implemented__, a new Implements instance is created and it is assigned to the newly generated request class as its __implemented__ attribute.
• Apparently a global registry of all classes to their implemented interface specification object (the Implements instance) is mutated when the above step happens (probably some sort of cache). I haven't tracked it down quite this far though.
• Because a new class is created on every request, the global registry grows without bound.

I don't know what the solution is yet.

[mcdonc]

(NB: thanks Wiggy for the info in the gc tween tracker; it allowed me to put pdb.set_trace calls in the right places to track this down)

[mcdonc]

One "global registry" might be the Implements object attached to the pyramid.request.Request class. As the result of the __setBases method of zope.interface.Specification (which is a base class of Implements, and is called as the result of a new Implements object being created), the dependents attribute of the Implements object for pyramid.request.Request gets a new key which is the newly created class (e.g. `self.dependents[] = ....``.). This is supposed to be a "weak key dictionary", so once the class is destroyed (presumably when the request ends), it should no longer be referenced, but maybe it doesn't work. Or something.

[mcdonc]

I've fixed this without understanding the issue in its entirety (without tracking down exactly how references are leaked) but the fix works.

[murrayreadccdc]

The fix seems to work ok for the minimal pyramid application. Unfortunately not for my real pyramid app. I will investigate more.

[digitalresistor]

@murrayreadccdc Could you provide more information on your memory leak?

[murrayreadccdc]

@bertjwregeer - it's been a year since I look at this, but my last note on this issue was "Using Pyramid 1.5b1 on Linux, I am not seeing significant evidence of memory leak now."