Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There's a retain loop between SubscriptionListener and ListenerToken if listener is added by SubscriptionSession.add(_:ListenerType) #92

Closed
AirBlack opened this issue Dec 27, 2021 · 14 comments · Fixed by #124
Labels
status: done This issue is considered resolved.

Comments

@AirBlack
Copy link

image
Screenshot 2021-12-27 at 17 30 28

SubscriptionListener has a strong reference to SubscriptionToken, SubscriptionToken has a strong reference to SubscriptionListener through it's cancellation closure capture group.

@parfeon parfeon added the status: accepted This issue is accepted as valid, but it's not being worked on yet. label Dec 31, 2021
@dterekhov
Copy link

It crashed for me at the same line: self?.privateListeners.remove(listener)
Here is a hint of how it was precisely caught: before the app was in the state sceneWillResignActive and on switching to sceneDidBecomeActive it crashed.

Screenshot 2022-12-12 at 12 56 16 AM

@AirBlack
Copy link
Author

It didn't crash for me but cos of retain loop and caching the subscription didn't clean itself and was stuck providing the app with duplicate/invalid values. I fixed it by somehow cleaning the subscription manually. I will post a workaround when I'll be next time around my working device.

@parfeon
Copy link
Contributor

parfeon commented Dec 22, 2022

There will be fix for this, so as soon as subscription listener instance will go out of scope, it will be removed from list of listeners and free-up resources properly.

@pubnub-release-bot
Copy link

@AirBlack this issue is addressed in 6.0.3

@github-actions github-actions bot added status: done This issue is considered resolved. and removed status: accepted This issue is accepted as valid, but it's not being worked on yet. labels Jan 5, 2023
@luisescamilla13
Copy link

luisescamilla13 commented Feb 14, 2023

Still getting the error with 6.0.3 :(
In $specialized Set._Variant.update(with:)

@parfeon
Copy link
Contributor

parfeon commented Feb 15, 2023

@luisescamilla13 hello. What exactly error do you get?
This issue was about retain cycle.

@parfeon
Copy link
Contributor

parfeon commented Feb 15, 2023

@luisescamilla13 is it possible to show place. where you experience issue in same way, as shown in first message?

@luisescamilla13
Copy link

@parfeon sure
This is the crash:

SIGSEGV : Crash due to signal: SIGSEGV(SEGV_MAPERR) at 00000010
$specialized Set._Variant.update(with:) (<compiler-generated>:0 )

This is the code where I initialize pubnub:

        // Setup PNConfiguration
        var configuration = PubNubConfiguration(publishKey: keys.publishKey, subscribeKey: keys.subscriptionKey, uuid: email)
        configuration.authKey = keys.token
        pubnub =  PubNub(configuration: configuration)
        listener = SubscriptionListener(queue: .main)
        pubnub.add(listener)

And here is the stack trace:
Screen Shot 2023-02-15 at 11 19 20

@parfeon
Copy link
Contributor

parfeon commented Feb 15, 2023

@luisescamilla13 do you store somewhere reference on listener? Without reference to it, the object will be deallocated right away.

@luisescamilla13
Copy link

luisescamilla13 commented Feb 15, 2023

@parfeon yes sir, my class has the following variable
let listener: SubscriptionListener
And the value is initialized in the init of my class

@parfeon
Copy link
Contributor

parfeon commented Feb 15, 2023

@luisescamilla13 is there any chance to have some minimum app which is able to reproduce this issue? With memory leak, we were able to reproduce it and fix, but I can't reproduce this one.

@luisescamilla13
Copy link

@parfeon to be honest I haven't been able to reproduce it by myself, we just get this random crash every two or three weeks from different users. I'll let you know if I get more details. Thanks for the response.

@parfeon
Copy link
Contributor

parfeon commented Feb 15, 2023

@luisescamilla13 thank you!
If possible, can you create a new issue, so we won't bother issue author on a non-related topic?

@dterekhov
Copy link

Continue getting crashes with the 6.0.5 latest version.
Please pay attention that the app crashes on com.apple.root.user-initiated-qos.cooperative thread + when going to the background state.

Screenshot 2023-05-30 at 7 07 04 PM Screenshot 2023-05-30 at 7 07 37 PM

cc @parfeon
Can you please re-open this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: done This issue is considered resolved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants