remote config realtime listeners not removed #11458
Comments
|
I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight. |
|
Relative to my log entries, this method in particular is what makes me think something is happening with the address of firebase-ios-sdk/FirebaseRemoteConfig/Sources/RCNConfigRealtime.m Lines 686 to 701 in 671c879 at line 688 the object is
But |
|
Here's the file with the statements I added so you can drop it in for repro perhaps master...mikehardy:firebase-ios-sdk:patch-1 |
|
Okay, if I move what is at line 695 down out of the - (FIRConfigUpdateListenerRegistration *)addConfigUpdateListener:
(void (^_Nonnull)(FIRRemoteConfigUpdate *configUpdate, NSError *_Nullable error))listener {
if (listener == nil) {
return nil;
}
NSLog(@"FirebaseRemoteConfig addConfigUpdateListener called on %@", listener);
__weak RCNConfigRealtime *weakSelf = self;
dispatch_async(_realtimeLockQueue, ^{
__strong RCNConfigRealtime *strongSelf = weakSelf;
/// cannot add the listener here, it is the wrong address and remove will not work?
///[strongSelf->_listeners addObject:listener];
NSLog(@"FirebaseRemoteConfig addConfigUpdateListener added %@, count now %d", listener, strongSelf->_listeners.count);
[strongSelf beginRealtimeStream];
});
/// new temporary variable here so we can access properties of the new registration
FIRConfigUpdateListenerRegistration *foo = [[FIRConfigUpdateListenerRegistration alloc] initWithClient:self
completionHandler:listener];
NSLog(@"FirebaseRemoteConfig addConfigUpdateListener returning FIRConfigUpdateListenerRegistration %@", foo);
/// now add the listener via the property that we know we will try to remove later. Listener callbacks still work, remove works
[self->_listeners addObject:foo.completionHandler];
return foo;...but then there's a new problem. Now the stream is paused but it doesn't want to start back up again. So if I have a test that removes the last listener - causing the stream to pause - then immediately add a new listener, the stream doesn't reconnect. I verified that was the next issue by commenting out the ...and things started working again across listener add/remove/add, though this is obviously not optimal as now there is no way for the developer to shut the socket down when not in use. |
|
Hey @mikehardy thanks for bringing this to our attention. Let us take a look at this and get back to you. |
|
I think the above PR should address the issue. The problem that I found was that changes happening within the dispatch_async block were not consistent with what is outside of the block. Making a copy with __block looks to address it. |
|
left notes on the PR, but I tested the PR and it does resolve my issue. Excellent |
Description
It appears something is happening with the object equality of the listeners such that they are registered, and you can iterate over them with updates correctly sent, but when you go to remove them the listener is not found in the listeners set so it is not actually removed
I noticed odd behavior during the react-native-firebase implementation (invertase/react-native-firebase#7119) where I was getting more events than expected
I reached this conclusion by adding a bunch of logging prefixed with
FirebaseRemoteConfigsee snippet belowHypothesis - either the wrong object is sent in to the remove call so it matches no listeners, or the listener address itself is somehow mutated while going into the dispatch_async block such that the set of listeners has slightly different addresses for the listener than that which is encapsulated in the registration's completion handler and thus isn't equal later when calling remove
Reproducing the issue
I think you can just add a listener, check the listeners count, remove the listener, then check the count again.
Firebase SDK Version
10.11.0
Xcode Version
14.3.1
Installation Method
CocoaPods
Firebase Product(s)
Remote Config
Targeted Platforms
iOS
Relevant Log Output
If using Swift Package Manager, the project's Package.resolved
Expand
Package.resolvedsnippetReplace this line with the contents of your Package.resolved.If using CocoaPods, the project's Podfile.lock
Expand
Podfile.locksnippetThe text was updated successfully, but these errors were encountered: