centrifugal / centrifuge-swift

Swift client SDK for bidirectional real-time communication with Centrifugo and Centrifuge-based server over WebSocket
MIT License
47 stars 41 forks source link

Subscription keep a strong reference to it's Client #21

Closed NailxSharipov closed 4 years ago

NailxSharipov commented 4 years ago

In one side a client keeps strong all it's subscriptions in other side a subscription is keeping the client.

We have a strong reference circle here, which cause a memory leak problems in some cases.

I don't see any reason why the subscription must keep a strong reference on it's client.

May be will be better if a subscription keeps a weak reference on it's client? Minus one case to catch a memory leak.

FZambia commented 4 years ago

@NailxSharipov I suppose this makes sense. Will it be even better if we replace client with delegate class? I am pretty unexperienced with mobile development - so this code definetely can contain some non-idiomatic things I'd happy to fix.

NailxSharipov commented 4 years ago

By the delegate do you mean a variable name or a protocol which client will be implemented with?. If it's a name as for me it doesn't matter. If it's a Protocol(an Interface), I think it useless there case delegate means that you have a lot of implementations of it. But we have just one private instance. I suppose it just should be replace with:

weak var centrifuge: CentrifugeClient?

and make the relevant fixes

FZambia commented 4 years ago

Yep, let's make it the way you suggest then. Could you please send pr?

NailxSharipov commented 4 years ago

resolved