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

iOS: Many crashes due to lack of thread-safety #1696

Closed
mars-lan opened this issue May 15, 2022 · 11 comments · Fixed by #2604
Closed

iOS: Many crashes due to lack of thread-safety #1696

mars-lan opened this issue May 15, 2022 · 11 comments · Fixed by #2604
Labels
🐛 bug Something isn't working 📱 iOS Related to iOS

Comments

@mars-lan
Copy link
Contributor

Please use Discussion board if you want get some help out of it. Please report issue if there is a bug.

Version of react-native-iap

8.0.8

Version of react-native

0.67.4

Platforms you faced the error (IOS or Android or both?)

iOS

Expected behavior

No crashing

Actual behavior

Random crashes with stack trace like this:

Crashed: com.facebook.react.RNIapIosQueue
EXC_BAD_ACCESS KERN_INVALID_ADDRESS 0x0000000000000000

<compiler-generated> - Line 388412 RNIapIos.addPromise(forKey:resolve:reject:) + 388412
RNIapIos.swift - Line 165 RNIapIos.getItems(_:resolve:reject:) + 165
<compiler-generated> - Line 391604 @objc RNIapIos.getItems(_:resolve:reject:) + 391604

Tested environment (Emulator? Real Device?)

Real device

Steps to reproduce the behavior

This is caused by non-thread-safe access to the dictionary promisesByKey. A serial queue was added to protect some of these accesses but not all. IMHO a better way to protect this is by adding dispatch directly inside of addPromise, resolvePromises & rejectPromises.

@andresesfm andresesfm added 🍗 enhancement New feature or request 📱 iOS Related to iOS labels Jun 6, 2022
@edodusi
Copy link

edodusi commented Jun 22, 2022

I have those crashes too with 8.1.3 and also this error which I think it's related:

EXC_BAD_ACCESS KERN_PROTECTION_FAILURE 0x0000005f774d5340

...
0x5ed518 RNIapIos.resolvePromises(forKey:value:) + 100 (RNIapIos.swift:100)
0x5f59f0 RNIapIos.productsRequest(_:didReceive:) + 446 (RNIapIos.swift:446)
0x5f5bb8 @objc RNIapIos.productsRequest(_:didReceive:) + 68 (<compiler-generated>:68)
0x3a18 __27-[SKProductsRequest _start]_block_invoke_2 + 188
...

@edodusi
Copy link

edodusi commented Jun 22, 2022

(also this should be labeled as bug I think)

@mars-lan mars-lan added 🐛 bug Something isn't working and removed 🍗 enhancement New feature or request labels Jun 22, 2022
@aadityapaliwal94
Copy link

Can anyone look into this issue? Causing crash on production.

@phillip-kic
Copy link

I believe we may be having issues related to this as well. We are seeing crashes on release builds which is preventing us from upgrading in production.

This is occurring when fetching subscription products. The Apple Docs warn that the SKProductsRequestDelegate may not return on a specific thread so feels likely that lack of thread-safety is the issue here?

Stack trace

Hardware Model:     iPhone13,4
Role:               Foreground
OS Version:         iOS 15.5
Exception Type:     EXC_BAD_ACCESS 
Exception Subtype:  KERN_INVALID_ADDRESS


EXC_BAD_ACCESS: Attempted to dereference garbage pointer 0x8000000000000028.

0  App Name                specialized __RawDictionaryStorage.find<A>(_:) (<compiler-generated>)
1  App Name                specialized Dictionary._Variant.removeValue(forKey:) (<compiler-generated>)
2  App Name                specialized Dictionary.removeValue(forKey:) (<compiler-generated>)
3  App Name                RNIapIos.productsRequest(_:didReceive:) (RNIapIos.swift:451:5)
4  App Name                @objc RNIapIos.productsRequest(_:didReceive:) (<compiler-generated>)
5  StoreKit                __27-[SKProductsRequest _start]_block_invoke_2
6  libdispatch.dylib       _dispatch_call_block_and_release
7  libdispatch.dylib       _dispatch_client_callout
8  libdispatch.dylib       _dispatch_queue_override_invoke
9  libdispatch.dylib       _dispatch_root_queue_drain
10 libdispatch.dylib       _dispatch_worker_thread2
11 libsystem_pthread.dylib _pthread_wqthread
12 libsystem_pthread.dylib start_wqthread

@andresesfm
Copy link
Contributor

Not sure but I think this: #1910 might have fixed some memory leaks related to this. If not, please reopen

@mars-lan
Copy link
Contributor Author

I don't think #1910 addressed the issue as this is to do with thread safety, not memory leaks. Keeping this re-opened for now.

@mars-lan mars-lan reopened this Sep 12, 2022
@phillip-kic
Copy link

Due to the Google Play Billing Client depreciation we have to upgrade our in production use of react native iap (v6), but every attempt we run into serious crashes such as this one now.

Can anyone confirm if upgrading to Version 7 (First version with Billing Client 4) is safe in production, or if there is a specific version that anyone is using successfully and do not see these crashes?

@phillip-kic
Copy link

For anyone else in this issue, we upgraded successfully to version 7. So version 7 does not have this issue, it was introduced in version 8 and that version should be avoided

@andresesfm
Copy link
Contributor

Please create a new ticket if this is still happening on 11.x.x. Also please use the new Bug report template. There's simply not enough information here to try to reproduce. I'll be happy to look at it.

@openpathgit
Copy link

This issue is causing our company to move off this library, which is a real shame. Please let us know the path moving forward

hyochan pushed a commit that referenced this issue Jun 12, 2024
**Fixes:** #1696, #2372

Similar to #2518, this fixes race condition crashes on `validProducts` and `productsRequest` in `RNIapIos.swift` when using Store Kit 1.

## React Native IAP
### What was done:
- **Created `ThreadSafe` class:** This now wraps `validProducts`.
- **Created `LatestPromiseKeeper`:**
  - Maintains only the latest promise to fix racing conditions when multiple `productsRequest` are made in parallel.
  - Resolves only one promise at any given time (for `productsRequest`), eliminating the possibility of racing conditions or execution exceptions while storing the resolved data.
  - Discards older promises if more than one is made, returning a rejection for the discarded promises.

## IAPExample (iOS)
- Fixed target iOS deployment being loaded from an undefined variable. It now fetches the project's target iOS from the Xcode project and applies it during pod install.
- Updated GitHub Action to use Node.js 20.
- Changed GitHub Action runner from M1 architecture to Intel-based macOS (fixes crash during build).
@arthurgeron
Copy link
Contributor

The recently published version 12.4.0 includes a fix for a racing condition in iOS with StoreKit 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 📱 iOS Related to iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants