[FR] Is FIRConfigUpdateListenerRegistration thread-safe? If not, can it be made thread-safe and Sendable?

[ehyche]

Description

I am creating an extension on RemoteConfig to provide an AsyncStream API on top of addOnConfigUpdateListener():

extension RemoteConfig {

    public static func onConfigUpdateThrowingStream() -> AsyncThrowingStream<Set<String>, any Error> {
        AsyncThrowingStream { continuation in
            let registration = RemoteConfig.remoteConfig().addOnConfigUpdateListener { update, error in
                if let error {
                    continuation.finish(throwing: error)
                } else if let update {
                    continuation.yield(update.updatedKeys)
                }
            }
            let registrationSendable = ConfigUpdateListenerRegistrationSendable(registration: registration)
            continuation.onTermination = { _ in
                registrationSendable.registration.remove()
            }
        }
    }
}

struct ConfigUpdateListenerRegistrationSendable: @unchecked Sendable {
    let registration: ConfigUpdateListenerRegistration
}

As you can see, since ConfigUpdateListenerRegistration is not Sendable, then I am wrapping it in an @unchecked Sendable struct.

From looking at the code, this appears to be safe to me, since in the implementation of the ConfigUpdateListenerRegistration.remove() method:

- (void)remove {
  [self->_realtimeClient removeConfigUpdateListener:_completionHandler];
}

which in turn does an operation on a serial queue:

- (void)removeConfigUpdateListener:(void (^_Nonnull)(FIRRemoteConfigUpdate *configUpdate,
                                                     NSError *_Nullable error))listener {
  __weak RCNConfigRealtime *weakSelf = self;
  dispatch_async(_realtimeLockQueue, ^{
    __strong RCNConfigRealtime *strongSelf = weakSelf;
    [strongSelf->_listeners removeObject:listener];
    if (strongSelf->_listeners.count == 0) {
      [strongSelf pauseRealtimeStream];
    }
  });
}

So it appears to me that ConfigUpdateListenerRegistration should be thread-safe. If so, can we please mark the type as Sendable so that I don't have to do this @unchecked Sendablewrapper to avoid the Swift 6 errors?

API Proposal

Just mark the ConfigUpdateListenerRegistration type as Sendable.

Firebase Product(s)

Remote Config

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[paulb777]

@ehyche Thanks for the issue. We've added the Sendable annotation to ConfigUpdateListenerRegistration for our next release.

Separately, we'll also consider adding AsyncStream based listening APIs to the SDK, probably in conjunction with or after reimplementing FirebaseRemoteConfig in Swift.