Hi! We noticed a somewhat rare crash that we’ve only recently been able to reproduce. This affects all iOS SDK version up to 4.17.5 (the latest at the time of writing). Basically we get a hard crash on
[[RCPurchases sharedPurchases] purchasePackage...];
The call stack simply points to libobjc.A.dylib at objc_msgSend within our method that calls purchasePackage.
We haven’t noticed this crash with the restorePurchasesWithCompletion call. The crash doesn’t happen with getOfferingsWithCompletion either.
We were finally able to reproduce this crash in the past few days with our TestFlight builds (and ONLY with TestFlight builds). We are calling all 3 methods from a thread that is not the main thread. Since only purchasePackage crashes (and on Android there are no crashes at all on any of the methods when not called from the UI thread), we never considered this to be an issue. However, then we queued the call the the main thread in iOS like this:
[[RCPurchases sharedPurchases] purchasePackage...];
And it appeared that the crashes stopped.
We’re not sure whether we’re the ones at fault for not calling the methods from iOS’s main thread or whether there is a bug in the SDK, but a solution would be appreciated (either in the form of a fix for the SDK or mentioning in the documentation that RC calls should always be done from the main thread).
Can somebody confirm this issue?
Catching up with this. Are you able to reproduce locally? Do you know which line in that function is actually causing the crash?
Thanks for re-sharing the stacktrace. I’m able to see it now.
Oh, and I’d missed that the lock was tied to the scope, my apologies.
As for the ref count, your variable declaration is already increasing the ref count, so that should be good. It would only have been an issue if the variable had been declared as weak, unowned or some other language equivalent.
The way it’s supposed to go is that the SDK calls a completion block and passes in an `RCOfferings` object as a parameter, but it doesn’t necessarily keep a reference to it afterwards (it might temporarily keep one for cache purposes, but it shouldn’t modify the existing instance). So if your code uses that object beyond the scope of the completion block, it will have to keep a reference to it, which you're already doing.
I’m honestly surprised to hear about differences between launching from App Store and Home screen, as far as I know those are supposed to be equivalent from an app lifecycle perspective.
I’m going to dig into this, to make sure that our SDK isn’t modifying that object and potentially breaking that reference afterwards. Especially given that you had also mentioned that iterating through availablePackages seemed to crash too.
We haven’t had other reports of this, so I’m inclined to think there’s something in the app code that’s triggering it. Could you share all the related code in the completion of getOfferingsWithCompletion? Is it just the two lines that you mentioned previously? Where does the validation of availablePackages happen?
Thanks for your patience on this.
The hmutex::ScopeLock object is used to ensure mutexes are always unlocked when exiting scope (though they can be released manually within current the scope as well). In case an exception is thrown somewhere further down the call hierarchy (which is then caught further up), this ensures that the mutex won’t get stuck in a locked state when exiting the current scope. This is a common technique in handling mutexes.
Through further testing it does appear that there are no threading issues after all.
The difference of running from App Store and Home dashboard is exactly what it sounds like. Running it from the Home screen means tapping on the app icon on the Home screen. Running from the App Store means navigating to the app page where the app can be downloaded and installed. If the app is already installed, App Store displays an “Open” button that can launch the app directly from that location.
As far as reference counting goes: We have a static RCOffering* variable within a namespace scope as mentioned above:
We assign that variable during the getOfferingsWithCompletion callback (of course, after checking for errors and verifying that offerings and offerings.current aren’t nil):
After that we simply use the rcOffering variable normally when needed. We don’t actually release it manually except at app shutdown.
Maybe we need to increase the ref count of that variable manually when we cache it? There was no indication in the docs that this is necessary.
I’m digging into this, but the first thing that called my attention was that the early return when a request fails doesn’t unlock the mutex, perhaps that’s causing some unintended side effects?
FWIW you should definitely be able to call any SDK methods from any threads without unexpected behaviors. Completion blocks are always executed on the main thread.
Also could you clarify the difference between running from App Store vs Home dashboard? I’m not sure what that means.
How and when are you setting your cached RCOffering object? The SDK won’t modify the value that was returned, but if the object got deallocated app-side (like if its ref count gets to zero), then that would cause a crash.
Sorry for the delay here, I created an internal ticket and will ping
@Andy, one of our mobile engineers, for any insight here. Thanks for the detailed post, we’ll take a look and update here soon.
We were finally able to reproduce the issue in a debug build. Our cached RCOffering object would have a EXC_BAD_ACCESS crash when attempting to access the availablePackages member. This implies that the variable has been deleted in the meantime. Is this intended? There is no mention in the documentation that keeping a pointer to the offerings.current object received from getOfferingsWithCompletion is hazardous.
The crashes have become a lot more frequent in SDK versions 4.17.5 and even more frequent in 4.17.7. This has become a serious issue for us now. We’re seeing significant reduction in revenue already and we’re considering removing RevenueCat altogether.
Any news yet?
We have some new information on this crash and it might not be a problem on our end. The top of the stack trace has changed to this:
This is the method where it crashes:
Notice how it actually crashes before it enters the _requestPurchaseInternal() method. The variable rcOffering is a static Objective-C variable outside of this C++ class.
The offeringMutex member is locked each time rcOffering is accessed so access is definitely synchronized and there should be no problems with access of this variable from any thread. This is what we do inside getOfferingsWithCompletion.
Or are we doing something wrong here? We’re checking whether rcOffering was received, whether its availablePackages member is valid and we attempt to fetch a package (we used to have manual iteration through availablePackages before, but it crashed somewhere here as well). Could any of these statements cause this crash? Or is it possible that we have to redo the getOfferingsWithCompletion before calling purchasePackage as rcOffering becomes stale?
We also noticed that this crash happens much more often when running the app directly from the App Store rather from the Home dashboard on iOS. We also turned on StoreKit 2, but it made not difference. The crash started happening a lot more once iOS 15 was released (with no reduction in iOS 16). So we’re been having this problem for quite a while now.
The stacktrace is attached. The crash happened in Thread 3#0.
Our engine runs on C++ with Java / Obj-C glue code where necessary. Usually we encapsulate this glue code in separate libraries so we can use pure C++ in our apps and games while being platform-agnostic. In this case crcat is the library that wraps the RevenueCat SDK. Hence the crash within the crcat::Manager_Apple::_requestPurchase() call.
We are aware that RC SDK doesn’t guarantee thread safety, but all calls happen on the same thread (but it’s not the main thread). So we expected that thread-safety shouldn’t be a concern.
Thanks for bringing this up. I don’t think the RevenueCat SDK makes any guarantees of being thread-safe, so for now I recommend that you run it on the main thread until I get some confirmation here.
Can you share the stack trace by any chance? I can pass it to our mobile engineers to look into.