Hello,

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.

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.

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:

Thread 3
#0    (null) in CoreFoundation ___forwarding___ ()
#1    (null) in CoreFoundation _CF_forwarding_prep_0 ()
#2    (null) in crcat::Manager_Apple::_requestPurchase(hltypes::String const&) ()
...

This is the method where it crashes:

bool Manager_Apple::_requestPurchase(chstr packageId)
{
	hmutex::ScopeLock lock(&this->offeringMutex);
	if (rcOffering == nil || rcOffering.availablePackages == nil)
	{
		this->addResultPurchaseFail("Offering request has failed! Please make sure you have a working Internet connection.");
		return true;
	}
	RCPackage* currentPackage = arcOffering packageWithIdentifier:eNSString stringWithUTF8String:packageId.cStr()]];
	lock.release();
	if (currentPackage == nil)
	{
		this->addResultPurchaseFail("Could not find package '" + packageId + "'! Please make sure you have a working Internet connection.");
		return true;
	}
	this->_requestPurchaseInternal(currentPackage);
	return true;
}

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.

static RCOffering* rcOffering = nil;

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.

hmutex::ScopeLock lock(&this->offeringMutex);
rcOffering = offerings.current;

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.

Any news yet?

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.

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.

Hello,

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.

@Daygames Hi! I wasn’t able to access the stacktrace.txt file you shared earlier, I got a permissions issue. Could you grant access? 

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. 

@Andy I reattached the file to this post. I don’t see any permission options for files.

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:

static RCOffering* rcOffering = nil;

We assign that variable during the getOfferingsWithCompletion callback (of course, after checking for errors and verifying that offerings and offerings.current aren’t nil):

rcOffering = offerings.current;

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.

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. 

@Andy I’ve attached the entire file handling the code interface. The “addResult*” methods queue the obtained data in a thread-safe manner to the top-level update code of the library. The rest of the code should be readable as is.

@Daygames thanks for sending this over! We’re digging into it. We’ll update here when we have more info. 

@Daygames just letting you know we’re still digging into it!

Catching up with this. Are you able to reproduce locally? Do you know which line in that function is actually causing the crash?

@Andy  what ended up happening here? @Daygames  was the issue ever solved for you?

Also @Andy, according to this answer, callbacks are not guaranteed on the main thread:

@AppDev777 we do issue out all of the callbacks on the main thread, or at least try to, where we can’t discard the possibility of something being called in a background thread until we can move to MainActor. 

But for all intents and purposes you should usually be able to count on callbacks coming from the main thread. 

IIRC we weren’t able to reproduce, and since we didn’t hear back we assumed that this was fixed, @Daygames I’m happy to help if that wasn’t the case

A lot of my callbacks don’t come on the main thread. No problem, but that’s the case for me. I’m using Objective-C, so that may be the reason.

Obj-C _shouldn’t_ be an issue, but let’s try to figure it out! 

A couple of questions: 

Which SDK version are you using? Which methods is this happening for? Could you share code snippets / debug logs? 

As mentioned before, we fixed it by making sure all calls are queued to the main thread. But we did suggest that this requirement (running all calls on the main thread) should be mentioned in the docs (if this is actually the case).

@Daygames thanks for getting back to us. We’ll do another pass at making sure that we’re not calling callbacks on threads that aren’t main, that’s what we should always be doing in any case so developers don’t have to.