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

Remote Config's addOnConfigUpdateListener crashing with Swift 6 set as Swift Language Version #14257

Open
GoofSports opened this issue Dec 13, 2024 · 9 comments · Fixed by #14314
Assignees
Milestone

Comments

@GoofSports
Copy link

Description

  1. Firebase's Remote Config will crash when using with addOnConfigUpdateListener EXC_BREAKPOINT and corrupt remote config's data. This makes it impossible to use addOnConfigUpdateListener with swift 6.

Reproducing the issue

To reproduce the issue, download this sample remote config project. https://github.com/TharinduKetipe/RemoteConfigDemo/tree/develop

After downloading, run the app as is with Swift 5. The app will run correctly. Go into build settings and change Swift Language Version to Swift 6. Run the app again, and there is an immediate crash.

Firebase SDK Version

11.6.0

Xcode Version

16.1

Installation Method

Swift Package Manager

Firebase Product(s)

Remote Config

Targeted Platforms

iOS

Relevant Log Output

if (completion) {
   dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
      completion(NO, nil); // EXC_BREAKPOINT HERE
   });
}

If using Swift Package Manager, the project's Package.resolved

Expand Package.resolved snippet
{
  "pins" : [
    {
      "identity" : "abseil-cpp-binary",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/abseil-cpp-binary.git",
      "state" : {
        "revision" : "194a6706acbd25e4ef639bcaddea16e8758a3e27",
        "version" : "1.2024011602.0"
      }
    },
    {
      "identity" : "app-check",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/app-check.git",
      "state" : {
        "revision" : "21fe1af9be463a359aaf8d96789ef73fc3760d09",
        "version" : "11.0.1"
      }
    },
    {
      "identity" : "firebase-ios-sdk",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/firebase-ios-sdk.git",
      "state" : {
        "revision" : "1fc52ab0e172e7c5a961f975a76c2611f4f22852",
        "version" : "11.2.0"
      }
    },
    {
      "identity" : "googleappmeasurement",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleAppMeasurement.git",
      "state" : {
        "revision" : "07a2f57d147d2bf368a0d2dcb5579ff082d9e44f",
        "version" : "11.1.0"
      }
    },
    {
      "identity" : "googledatatransport",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleDataTransport.git",
      "state" : {
        "revision" : "617af071af9aa1d6a091d59a202910ac482128f9",
        "version" : "10.1.0"
      }
    },
    {
      "identity" : "googleutilities",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/GoogleUtilities.git",
      "state" : {
        "revision" : "53156c7ec267db846e6b64c9f4c4e31ba4cf75eb",
        "version" : "8.0.2"
      }
    },
    {
      "identity" : "grpc-binary",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/grpc-binary.git",
      "state" : {
        "revision" : "f56d8fc3162de9a498377c7b6cea43431f4f5083",
        "version" : "1.65.1"
      }
    },
    {
      "identity" : "gtm-session-fetcher",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/gtm-session-fetcher.git",
      "state" : {
        "revision" : "a2ab612cb980066ee56d90d60d8462992c07f24b",
        "version" : "3.5.0"
      }
    },
    {
      "identity" : "interop-ios-for-google-sdks",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/interop-ios-for-google-sdks.git",
      "state" : {
        "revision" : "2d12673670417654f08f5f90fdd62926dc3a2648",
        "version" : "100.0.0"
      }
    },
    {
      "identity" : "leveldb",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/leveldb.git",
      "state" : {
        "revision" : "a0bc79961d7be727d258d33d5a6b2f1023270ba1",
        "version" : "1.22.5"
      }
    },
    {
      "identity" : "nanopb",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/firebase/nanopb.git",
      "state" : {
        "revision" : "b7e1104502eca3a213b46303391ca4d3bc8ddec1",
        "version" : "2.30910.0"
      }
    },
    {
      "identity" : "promises",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/google/promises.git",
      "state" : {
        "revision" : "540318ecedd63d883069ae7f1ed811a2df00b6ac",
        "version" : "2.4.0"
      }
    },
    {
      "identity" : "swift-protobuf",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/apple/swift-protobuf.git",
      "state" : {
        "revision" : "edb6ed4919f7756157fe02f2552b7e3850a538e5",
        "version" : "1.28.1"
      }
    }
  ],
  "version" : 2
}

If using CocoaPods, the project's Podfile.lock

Expand Podfile.lock snippet
Replace this line with the contents of your Podfile.lock!
@google-oss-bot
Copy link

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

@ErikLuimes
Copy link

We have the same issue

@paulb777 paulb777 assigned paulb777 and unassigned ncooke3 Dec 23, 2024
@paulb777
Copy link
Member

The repo doesn't build for me on Xcode 16.2

Image

If I add @preconcurrency to RemoteConfigProvider, the app starts up without crashing

@ErikLuimes
Copy link

ErikLuimes commented Dec 24, 2024

Below is a stacktrace, I can't reproduce it in the simulator or on my own device but we see some of these crashes in our Beta group. It happens when the remote update closure is called

remoteConfig.addOnConfigUpdateListener { config, error in
        
}
Crashed: com.google.GoogleConfigService.FIRRemoteConfig
0  libdispatch.dylib              0x64d8 _dispatch_assert_queue_fail + 120
1  libdispatch.dylib              0x6460 _dispatch_assert_queue_fail + 194
2  libswift_Concurrency.dylib     0x62b78 swift_task_isCurrentExecutorImpl(swift::SerialExecutorRef) + 284
3  APP                           0x259170 closure #2 in static AppSetup.setupRemoteConfig() + 4376236400 (<stdin>:4376236400)
4  APP                           0x259cd8 thunk for @escaping @callee_guaranteed (@guaranteed FIRRemoteConfigUpdate?, @guaranteed Error?) -> () + 4376239320 (<compiler-generated>:4376239320)
5  APP                           0xa1ff14 __37-[RCNConfigRealtime propogateErrors:]_block_invoke + 161 (RCNConfigRealtime.m:161)
6  libdispatch.dylib              0x2370 _dispatch_call_block_and_release + 32
7  libdispatch.dylib              0x40d0 _dispatch_client_callout + 20
8  libdispatch.dylib              0xb6d8 _dispatch_lane_serial_drain + 744
9  libdispatch.dylib              0xc1e0 _dispatch_lane_invoke + 380
10 libdispatch.dylib              0x17258 _dispatch_root_queue_drain_deferred_wlh + 288
11 libdispatch.dylib              0x16aa4 _dispatch_workloop_worker_thread + 540
12 libsystem_pthread.dylib        0x4c7c _pthread_wqthread + 288
13 libsystem_pthread.dylib        0x1488 start_wqthread + 8

@paulb777
Copy link
Member

@ErikLuimes That crash may have been addressed in the 11.5.0 release. See #13776 and #13825

@GoofSports
Copy link
Author

Hi @paulb777. Based on what I can tell, those issues still don't fix the crash as the crash is . As to your comment above about the repo not building, please annotate the RemoteConfigProvider class with '@mainactor' instead of '@preconcurrency' and it will compile and crash when you run. Please let me know if you need any more help.

@ncooke3
Copy link
Member

ncooke3 commented Jan 6, 2025

Because RemoteConfigProvider is marked with @MainActor, it's methods are isolated to the main actor by default.

func fetchCloudValues() { // ← Inherits main actor isolation from enclosing class
        remoteConfig.fetch { [weak self] (status, error) -> Void in
            guard let self = self else { return }
            
            if status == .success {
                self.remoteConfig.activate {  _, error in // ← The closure Inherits main actor isolation from enclosing method
                // Explicitly, this looks like: 
                // self.remoteConfig.activate {  @MainActor _, error in
                    if let error = error {
                        print(error.localizedDescription)
                        return
                    }
                    self.fetchComplete = true
                    print("Remote config fetch success")
                    DispatchQueue.main.async {
                        self.loadingDoneCallback?()
                    }
                }
            } else {
                print("Remote config fetch failed")
                DispatchQueue.main.async {
                    self.loadingDoneCallback?()
                }
            }
        }
    }

In the Remote Config implementation, the closure passed to the activate method is invoked on a utility thread (not the main thread).

@implementation FIRRemoteConfig {
// -- snip
- (void)activateWithCompletion:(FIRRemoteConfigActivateChangeCompletion)completion {
  // -- snip
  if (completion) {
  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
    completion(NO, nil); // ← Invoked on non-main thread.
  });
  // -- snip
}
// --snip

The closure is expected to run on the main thread/actor. When it is called from the Objective-C code, it's called off of the main thread, so Swift detects it at runtime as a possible data race and crashes to consistently expose the issue.

If RemoteConfigProvider needs to be marked @MainActor, one option is to explicitly mark the closure passed to the activate API as Sendable. This tells the compiler to disable the runtime isolation check as the closure can be safely sent to execute in any isolation domain.

self.remoteConfig.activate { @Sendable _, error in // ← Add explicit @Sendable to override implicit, inherited @MainActor isolation
    if let error = error {
        print(error.localizedDescription)
        return
    }
    self.fetchComplete = true // 💥 Main actor-isolated property 'fetchComplete' can not be mutated from a Sendable closure
    print("Remote config fetch success")
    DispatchQueue.main.async {
        self.loadingDoneCallback?()
    }
}

To fix the resulting compiler error, the main actor-isolated fetchComplete property needs to be updated back on the main actor/thread. This could be fixed by moving it into the main queue dispatch a couple lines down.

self.remoteConfig.activate { @Sendable _, error in // ← Add explicit @Sendable to override implicit, inherited @MainActor
    if let error = error {
        print(error.localizedDescription)
        return
    }

    print("Remote config fetch success")
    DispatchQueue.main.async {
        self.fetchComplete = true
        self.loadingDoneCallback?()
    }
}

This problem is more obvious when the library is written in Swift because the Swift compiler can warn about the closure dispatched to the utility queue not being Sendable:
Image

@oy-yeonhwas
Copy link

oy-yeonhwas commented Jan 14, 2025

@ncooke3 After the Firebase 11.7.0 update, we are still experiencing crashes in Swift 6.0 within the addOnConfigUpdateListener function, particularly in the remoteConfigUpdateCompletion closure.

The crash appears to be related to the listener located on line 468 of the RCNConfigRealtime.m file.

We would greatly appreciate it if you could investigate this issue and let us know if there are any updates or an estimated timeline for a fix. Thank you for your support!

Image

@ncooke3 ncooke3 reopened this Jan 14, 2025
@ncooke3
Copy link
Member

ncooke3 commented Jan 14, 2025

Thanks, @oy-yeonhwas, for the feedback. As a workaround until a fix is ready, you may be able to mark the closure passed to the API as Sendable.

E.g. with activate API

remoteConfig.activate { @Sendable _, error in // ← Add explicit @Sendable before closure params
  // ....
}

@ncooke3 ncooke3 added this to the 11.8.0 - M159 milestone Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants