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

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

Closed
aganov opened this issue Jul 2, 2021 · 8 comments · Fixed by #436
Closed

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

aganov opened this issue Jul 2, 2021 · 8 comments · Fixed by #436
Labels
need triage question Further information is requested

Comments

@aganov
Copy link
Collaborator

aganov commented Jul 2, 2021

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):

  • Device: iPhone 8 (sim), iPad mini 2 (real device)
  • OS: iOS 12.4
  • Stripe iOS SDK: 21.6.0
  • Stripe React Native SDK: 0.14

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
Copy link
Collaborator Author

aganov commented Jul 7, 2021

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
Copy link
Collaborator Author

aganov commented Jul 8, 2021

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 aganov changed the title CardField crash the app when field got focused on iOS 12.4 CardField crash the app with EXC_BAD_ACCESS on iOS Jul 8, 2021
@thorsten-stripe thorsten-stripe added need triage question Further information is requested labels Jul 8, 2021
@aganov
Copy link
Collaborator Author

aganov commented Jul 13, 2021

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.

Kapture.2021-07-13.at.09.10.38.mp4

@aganov aganov changed the title CardField crash the app with EXC_BAD_ACCESS on iOS CardField crash the app with EXC_BAD_ACCESS on iOS 13.* Jul 13, 2021
@aganov
Copy link
Collaborator Author

aganov commented Jul 13, 2021

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
Copy link
Collaborator

@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
Copy link
Collaborator Author

aganov commented Jul 13, 2021

@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
Copy link
Collaborator

@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
Copy link
Collaborator

This will be fixed from the next version (0.1.6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need triage question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants