stripe / stripe-react-native

React Native library for Stripe.
https://stripe.dev/stripe-react-native
MIT License
1.26k stars 262 forks source link

CardField crash the app with EXC_BAD_ACCESS on iOS 13.* #391

Closed aganov closed 3 years ago

aganov commented 3 years ago

Describe the bug <CardField /> cause a crash if field is not completed when focused previously and got unmounted later on. Happens only on iOS 12

To Reproduce Steps to reproduce the behavior:

  1. Create navigation stack with react-navigation
  2. Add <CardField /> component
  3. Focus on CardField without enter anything
  4. Go back navigation.goBack
  5. Navigate to the screen with CardField again
  6. Try to focus CardField
  7. App crashes with EXC_BAD_ACCESS

I'm expecting this to crash too if got unmounted and mounted again on the same screen, but have not tried it yet.

Expected behavior App not crashing

Smartphone (please complete the following information):

Additional context

Exception Type:     EXC_BAD_ACCESS 
Exception Subtype:  KERN_INVALID_ADDRESS

EXC_BAD_ACCESS: Attempted to dereference garbage pointer 0x3ff0000000000008.

0  QuartzCore       _CALayerGetSuperlayer
1  UIKitCore        -[UIView(UIKitManual) superview]
2  UIKitCore        -[UITextSelectionView removeFromSuperview]
3  UIKitCore        -[UITextSelectionView invalidate]
4  UIKitCore        -[UITextInteractionAssistant(UITextInteractionAssistant_Internal) dealloc]
5  UIKitCore        -[UITextField dealloc]
6  CoreFoundation   -[__NSDictionaryM removeAllObjects]
7  UIKitCore        -[UIKBAutofillController clearAutofillGroup]
8  UIKitCore        -[UIKBAutofillController _needAutofillCandidate:delegateAsResponder:]
9  UIKitCore        -[UIKBAutofillController needAutofillCandidate:delegateAsResponder:keyboardState:]
10 UIKitCore        -[UIKeyboardImpl needAutofillCandidate:]
11 UIKitCore        -[UIKeyboardImpl setDelegate:force:]
12 UIKitCore        -[UIPeripheralHost(UIKitInternal) _reloadInputViewsForResponder:]
13 UIKitCore        -[UIResponder(UIResponderInputViewAdditions) reloadInputViews]
14 UIKitCore        -[UIResponder becomeFirstResponder]
15 UIKitCore        -[UIView(Hierarchy) becomeFirstResponder]
16 UIKitCore        -[UITextField becomeFirstResponder]
17 UIKitCore        -[UITextInteractionAssistant(UITextInteractionAssistant_Internal) setFirstResponderIfNecessary]
18 UIKitCore        -[UITextSelectionInteraction oneFingerTap:]
19 UIKitCore        -[UIGestureRecognizerTarget _sendActionWithGestureRecognizer:]
20 UIKitCore        __UIGestureRecognizerSendTargetActions
21 UIKitCore        __UIGestureRecognizerSendActions
22 UIKitCore        -[UIGestureRecognizer _updateGestureWithEvent:buttonEvent:]
23 UIKitCore        __UIGestureEnvironmentUpdate
24 UIKitCore        -[UIGestureEnvironment _deliverEvent:toGestureRecognizers:usingBlock:]
25 UIKitCore        -[UIGestureEnvironment _updateForEvent:window:]
26 UIKitCore        -[UIWindow sendEvent:]
27 UIKitCore        -[UIApplication sendEvent:]
28 UIKitCore        ___dispatchPreprocessedEventFromEventQueue
29 UIKitCore        ___handleEventQueueInternal
30 UIKitCore        ___handleHIDEventFetcherDrain
31 CoreFoundation   ___CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
32 CoreFoundation   ___CFRunLoopDoSource0
33 CoreFoundation   ___CFRunLoopDoSources0
34 CoreFoundation   ___CFRunLoopRun
35 CoreFoundation   _CFRunLoopRunSpecific
36 GraphicsServices _GSEventRunModal
37 UIKitCore        _UIApplicationMain
38 ExampleApp      _mh_execute_header (ExampleApp)
39 libdyld.dylib    _start
aganov commented 3 years ago

We start to receive similar crash reports from the production app. I'm not 100% confident that this is coming from stripe-react-native, but those kinds of issues started to pop after migration from tipsi-stripe. Breadcrumb shows that every crash came after popToTop from screen with <CardField /> component. All crash reports with this issue are from iOS 13.x for now. Interesting part is that garbage pointer is aways 0x6565656565656569

Hardware Model:     iPhone11,2
Role:               Foreground
OS Version:         iOS 13.6.1
Exception Type:     EXC_BAD_ACCESS 
Exception Subtype:  KERN_INVALID_ADDRESS

EXC_BAD_ACCESS: Attempted to dereference garbage pointer 0x6565656565656569.

0  QuartzCore       CA::Layer::mask()
1  UIKitCore        -[UIView _setMaskView:]
2  UIKitCore        -[UIView dealloc]
3  UIKitCore        -[UIControl dealloc]
4  UIKitCore        -[UITextField dealloc]
5  UIKit            -[UITextFieldAccessibility dealloc]
6  libobjc.A.dylib  __object_remove_assocations
7  libobjc.A.dylib  _objc_destructInstance
8  libobjc.A.dylib  __objc_rootDealloc
9  QuartzCore       -[CALayer dealloc]
10 UIKit            -[CALayerAccessibility__UIKit__QuartzCore dealloc]
11 QuartzCore       CA::release_objects(X::List<void const*>*)
12 QuartzCore       CA::release_root_if_unused(CA::Layer*, CA::Layer*, void*)
13 QuartzCore       _x_hash_table_remove_if
14 QuartzCore       CA::Transaction::commit()
15 QuartzCore       CA::Transaction::observer_callback(__CFRunLoopObserver*, unsigned long, void*)
16 CoreFoundation   ___CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__
17 CoreFoundation   ___CFRunLoopDoObservers
18 CoreFoundation   ___CFRunLoopRun
19 CoreFoundation   _CFRunLoopRunSpecific
20 GraphicsServices _GSEventRunModal
21 UIKitCore        _UIApplicationMain
22 ExampleApp       _mh_execute_header (ExampleApp)
23 libdyld.dylib    _start
aganov commented 3 years ago

Removing removeFromSuperview from CardFieldView seems to fix the crash on iOS 12.4 and I'm suspecting that deallocation is not done properly or something. @thorsten-stripe can you help with this issue. It cause our app to randomly crash on 10-15% of the payments in production...

override func removeFromSuperview() {
    self.delegate?.onDidDestroyViewInstance(id: CARD_FIELD_INSTANCE_ID)
}
aganov commented 3 years ago

We've created example app https://github.com/aganov/stripe-rn-ca-mask-crash containing just react-navigation and stripe-react-native https://github.com/aganov/stripe-rn-ca-mask-crash/blob/master/App.js it crashes with EXC_BAD_ACCESS on CA::Layer::mask() every time you go back from the screen containing <CardField /> Please note that crash happens only on **iOS 13.*** like our production app.

https://user-images.githubusercontent.com/176610/125401219-8da93c00-e3bb-11eb-9a88-1ec0d3e144ed.mp4

aganov commented 3 years ago

Ok getting rid of strange cardFieldMap and cleaning up delegate stuff seems to fix the issue

// CardFieldManager.swift
@objc(CardFieldManager)
class CardFieldManager: RCTViewManager {
    public func getCardFieldReference(id: String) -> Any? {
        return nil
    }

    override func view() -> UIView! {
        return CardFieldView()
    }

    override class func requiresMainQueueSetup() -> Bool {
        return true
    }
}

Also I've added

// CardFieldView.swift
deinit {
    print("deinit CardFieldView")
}

to CardFieldView class and seems the CardFieldView is properly deallocated after unmount (stack navigation pop).

But now obviously StripeSdk methods won't be able to work without proper implementation of getCardFieldReference

@thorsten-stripe @arekkubaczkowski Any help on this would be appreciated. I'm thinking of passing cardParams directly to stripe methods or something but getCardFieldReference is deeply integrated into current implementation, so it would be better to reimplement it. I was considering something like this too, but without reactTag it's impossible to get the the current view.

public func getCardFieldReference(id: String) -> Any? {
    return self.bridge.uiManager.view(forReactTag: ????)
}
arekkubaczkowski commented 3 years ago

@aganov we are aware of this issue and where it comes from, unfortunately we cannot re-implement it because of PCI compliance. We're in the middle of looking for solution to correct card field deallocation. Thank you for providing such precise dubugging results.

aganov commented 3 years ago

@arekkubaczkowski Thanks for the update. Just an idea: Since now we have CardField ref we can pass it to createPaymentMethod

public func getCardFieldReference(node: NSNumber) -> Any? {
  // RCTExecuteOnMainQueue?
  return self.bridge.uiManager.view(forReactTag: node)
}
func createPaymentMethod(params, options) {
  let cardFieldView = cardFieldUIManager?.getCardFieldReference(node: options.cardFieldNode) as? CardFieldView  
}
const cardFieldRef = useRef(null)
<CardField ref={cardFieldRef} />
const result = await createPaymentMethod({ ... }, { cardFieldNode: findNodeHandle(cardFieldRef.current) })

or smth like this.

arekkubaczkowski commented 3 years ago

@aganov thanks for suggestion but we need an access to the card field params synchronously, RN bridge is async for now (until we don't use turbomodules)

arekkubaczkowski commented 3 years ago

This will be fixed from the next version (0.1.6)