Pylons / pyramid

Pyramid - A Python web framework
https://trypyramid.com/
Other
3.97k stars 887 forks source link

Break reference cycle between request and context. #3649

Closed luhn closed 3 years ago

luhn commented 3 years ago

When implementing traversal, it's common (at least for me) to store the request in the resource objects so you can do things like database lookups.

class MyResource:
    def __init__(self, request):
        self.request = request

    def __getitem__(self, key):
        if self.request.db.lookup(key):
            return ChildResource(self.request, key)
        else:
            raise KeyError()

I realized this morning that this will cause a reference cycle between request.context and context.request. Not the end of the world, obviously, but unnecessary pressure on the GC.

Maybe this is the user's responsibility? We could document not to store the request in the context, or to use a WeakRef.

The framework can also prevent this by breaking the cycle once the request has been processed, which is what this PR does. Really it's just a single line (request.__dict__.pop('context', None)), but it got a bit more complicated than that because several router tests rely on the context object being in the request.

mmerickel commented 3 years ago

LGTM, do you mind updating the changelog?

luhn commented 3 years ago

Done. I'll also backport to 1.10.

luhn commented 3 years ago

Anything more that needs to happen on this before merge?

@stevepiercy, I made the requested rst changes.

mmerickel commented 3 years ago

It's ready once @stevepiercy releases it.