Code review comment for lp:~therve/storm/reference-changed-leak

Thanks a lot for the review.

> [1]
> This seems to be relying on the fact that weakref.ref() always returns the
> same reference object given the same input. Looking at the weakref.__new__
> implementation this is indeed true, but I don't see it spelled out in the
> Python documentation.
>
> The source code makes it sound like an optimisation rather than a language
> feature, so I wonder if it is a good idea to rely on it?

Actually, I think it relies on the fact they have the same hash and are equals, which is made explicit in the documentation.

> [2]
> For on_remote references, an additional "removed" event handler is hooked in
> the remote -> local direction. Won't this cause the same circular reference
> problem that the remote -> local "changed" handler does?

Nice catch, I fixed it.

> [3]
> If we have references on both the local and remote objects (one forward, one
> backward) and access them both, will that generate the same kind of cycle?
> While each ref in isolation doesn't form a cycle, as a pair they do.

The thing is that we want (for now) to keep the objects alive if they have a direct reference. So, no, I don't think it's a problem, because the objects will stay alive only if at least one of both objects are referenced directly.