websocket.py: reference cycle between WebSocketProtocol and WebSocketHandler #382

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@WGH-
Contributor

WGH- commented Oct 13, 2011

Self-explanatory. I used weakref, if this practice is acceptable

@bdarnell

This comment has been minimized.

Show comment Hide comment
@bdarnell

bdarnell Oct 14, 2011

Owner

Are you sure this has the desired effect? Python's garbage collector handles cycles, it's just a little slower at reclaiming them than for non-cyclic references. I think (but haven't verified) that weak references still count as cyclic references as far as when they are reclaimed, so this may not cause the cycles to get cleaned up any faster.

Owner

bdarnell commented Oct 14, 2011

Are you sure this has the desired effect? Python's garbage collector handles cycles, it's just a little slower at reclaiming them than for non-cyclic references. I think (but haven't verified) that weak references still count as cyclic references as far as when they are reclaimed, so this may not cause the cycles to get cleaned up any faster.

@WGH-

This comment has been minimized.

Show comment Hide comment
@WGH-

WGH- Oct 14, 2011

Contributor

Yeah, gc does handle cycles. But I believe that cycles should be avoided if one is dealing with objects that are created repeatedly. For every single request, in this case. Using garbage collector makes memory usage pattern less predictable if we have a lot of requests.
If weak references didn't help to resolve reference cycles, they would be nearly useless. This is how I prove that weakref works :)
Jokes aside, I tried to the following:
>>> import gc
>>> gc.disable()
>>> class A: pass
>>> a = A()
>>> b = A()
>>> import weakref
>>> weakref.ref(a)
<weakref at 0x28625eb4; to 'instance' at 0x28624e8c>
>>> ref = _
>>> a.b = weakref.ref(b)
>>> b.a = weakref.ref(a)
>>> del a, b
>>> ref() is None
True
With almost the same code, but without weak references between a and b, that last ref() returned a.

Contributor

WGH- commented Oct 14, 2011

Yeah, gc does handle cycles. But I believe that cycles should be avoided if one is dealing with objects that are created repeatedly. For every single request, in this case. Using garbage collector makes memory usage pattern less predictable if we have a lot of requests.
If weak references didn't help to resolve reference cycles, they would be nearly useless. This is how I prove that weakref works :)
Jokes aside, I tried to the following:
>>> import gc
>>> gc.disable()
>>> class A: pass
>>> a = A()
>>> b = A()
>>> import weakref
>>> weakref.ref(a)
<weakref at 0x28625eb4; to 'instance' at 0x28624e8c>
>>> ref = _
>>> a.b = weakref.ref(b)
>>> b.a = weakref.ref(a)
>>> del a, b
>>> ref() is None
True
With almost the same code, but without weak references between a and b, that last ref() returned a.

@WGH-

This comment has been minimized.

Show comment Hide comment
@WGH-

WGH- Oct 15, 2011

Contributor

Also, with these two patches applied locally, handler objects were deallocated (at the next tick or something like that, because before that reference to on_connection_close was still alive) even with gc disabled altogether.

Contributor

WGH- commented Oct 15, 2011

Also, with these two patches applied locally, handler objects were deallocated (at the next tick or something like that, because before that reference to on_connection_close was still alive) even with gc disabled altogether.

@bdarnell

This comment has been minimized.

Show comment Hide comment
@bdarnell

bdarnell Oct 16, 2011

Owner

Cool, it's good to know that weak references don't force the objects onto the slow GC path. However, I'm still uneasy using them to "break cycles" in this way since it doesn't really fit the semantics of weak references. You're assuming that both objects will have the same lifetime, but that can be tricky to ensure (this is why httpclient's HTTPResponse and HTTPError have strong references to each other). I think it's best to either leave the cycle and trust the GC (remember that in e.g. pypy there is no refcounting fast-path, it's always GC), or refactor the code so the cycle is not necessary.

Owner

bdarnell commented Oct 16, 2011

Cool, it's good to know that weak references don't force the objects onto the slow GC path. However, I'm still uneasy using them to "break cycles" in this way since it doesn't really fit the semantics of weak references. You're assuming that both objects will have the same lifetime, but that can be tricky to ensure (this is why httpclient's HTTPResponse and HTTPError have strong references to each other). I think it's best to either leave the cycle and trust the GC (remember that in e.g. pypy there is no refcounting fast-path, it's always GC), or refactor the code so the cycle is not necessary.

@bdarnell bdarnell closed this in 3e4b4ca Apr 14, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment