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

Crash in ConfigurationCallbackStorage After Upgrading to 6.23.2 #1382

Closed
sadaf-behbahani opened this issue Aug 5, 2024 · 13 comments · Fixed by #1385
Closed

Crash in ConfigurationCallbackStorage After Upgrading to 6.23.2 #1382

sadaf-behbahani opened this issue Aug 5, 2024 · 13 comments · Fixed by #1385

Comments

@sadaf-behbahani
Copy link

sadaf-behbahani commented Aug 5, 2024

Braintree SDK Version

6.23.2

Environment

Both

Xcode Version

15.4

OS Version & Device

iOS 17.*

Integration type

Swfit Package Manager

Development Processor

Apple Silicon (M-series chips)

Describe the bug

We encountered a critical issue after upgrading Braintree from version 6.18.2 to 6.23.2. The crash occurs in ConfigurationCallbackStorage at line 16 when attempting to add a completion to the queue. Below is a detailed description of the problem:

  1. Create a BraintreeClient.
  2. During the initialization of BraintreeClient, we instantiate a BTAPIClient.
  3. Inside the BTAPIClient initialization, the method fetchOrReturnRemoteConfiguration is called without a completion handler, kicking off the background refresh and adding a completion to the pending completions. In our case, the internet is slow, and we don't get the configuration right away.
  4. We proceed to create a BTCardClient from the BTAPIClient instance.
  5. We then create a BTApplePayClient and start the payment process, which will call tokenize. This again calls fetchOrReturnRemoteConfiguration, adding a second completion to the pending completions.
  6. After this point, the configuration network call succeeds.
  7. ConfigurationCallbackStorage will sync/lock the queue and start invoking the completion blocks:
    • The first block works because it's a No-op.
    • The second block calls BTApplePayClient.tokenize, where the crash occurs. Tokenize calls self.apiClient.post, and inside the post function, it calls fetchOrReturnRemoteConfiguration.

Just a small reminder to ensure that a strong self is not captured inside of a block to avoid potential retain cycles and other issues.

To reproduce

Check the explanation above

Expected behavior

No crash should occur, and the completion handlers should be managed correctly.

Screenshots

No response

@dane-thomas-vs
Copy link

I have a project that is also experiencing this crash. We upgraded from 6.21.0 to 6.23.1 to resolve a different crash and now this one has started to occur. For us, it occurs when using the BTApplePayClient.tokenize method. Is there a safe version to downgrade to while this gets investigated? Please advise.

@sadaf-behbahani
Copy link
Author

So as I mentioned the version that worked for us is the 6.18.2

@scannillo
Copy link
Contributor

Hi @sadaf-behbahani - thank you for raising this to our attention. We are investigating it currently. Please try v6.23.0 for now.

Do you have any additional log or stack trace details on the crash?

@dane-thomas-vs
Copy link

Thank you for looking into the issue. Here is a stack trace for the crash we are experiencing.

BraintreeCrash

@jaxdesmarais
Copy link
Contributor

Thanks for sharing that stack @dane-thomas-vs -

Are you able to share an example of how you have your API and feature client set up in your view? We are hoping to replicate the crash you are seeing in our demo app.

@dane-thomas-vs
Copy link

Sure thing. We have a React Native application, so the SDK is wrapped in a Native Module. When we get the PKPayment object back from the Apple Pay sheet, we execute this code:

func paymentAuthorizationViewController(
        _ controller: PKPaymentAuthorizationViewController,
        didAuthorizePayment payment: PKPayment,
        handler completion: @escaping (PKPaymentAuthorizationResult) -> Void
    ) {
        self.paymentAuthorizationCompletion = completion
        
        guard let clientSecret = self.clientSecret,
                let apiClient = BTAPIClient(authorization: clientSecret) else {
            let error = BraintreeError.apiClientFailure
            self.eventDelegate?.sendEvent(onPaymentAuthorizedFailedEventName,
                                          ["error": error.toDict()])
            return
        }

        let client = BTApplePayClient(apiClient: apiClient)

        client.tokenize(payment, completion: { [weak self] (applePayNonce, error) in
            guard let self = self else { return }
            
            if let error = error {
                let braintreeError = BraintreeError.nonceError(error: error as NSError)
                self.eventDelegate?.sendEvent(onPaymentAuthorizedFailedEventName,
                                              ["error": braintreeError.toDict()])
                return
            }
            
            guard let nonceData = applePayNonce else {
                let braintreeError = BraintreeError.nonceError(error: nil)
                self.eventDelegate?.sendEvent(onPaymentAuthorizedFailedEventName,
                                              ["error": braintreeError.toDict()])
                return
            }
            
            var resultDict: [String: Any] = ["nonce": nonceData.nonce]
            
            resultDict["type"] = "applePay"

            if let billingContact = payment.billingContact {
                let mappedBilling = ApplePayUtils.mapFromContact(contact: billingContact)
                resultDict["billingContact"] =  mappedBilling
            }

            if let shippingContact = payment.shippingContact {
                let mappedShipping = ApplePayUtils.mapFromContact(contact: shippingContact)
                resultDict["shippingContact"] = mappedShipping
            }

            if let shippingMethod = payment.shippingMethod {
                let mappedMethod = ApplePayUtils.mapFromShippingMethod(shippingMethod: shippingMethod)
                resultDict["shippingMethod"] = mappedMethod
            }
            
            self.eventDelegate?.sendEvent(onPaymentAuthorizedSuccessEventName, resultDict)
        })
    }

The eventDelegate just sends that data back across the React native bridge. The completion handler is called at a later time. It seems the issue happens in the client.tokenize call.

@dane-thomas-vs
Copy link

I have some additional information. I noticed many of our error logs included the "Low Memory" warning. With that knowledge, I was able to reproduce the crash by simulating the memory pressure warning (using iOS simulator) after launching the Apple Pay sheet and approving the payment. Hope that helps!

@jaxdesmarais
Copy link
Contributor

Thanks you both for those details, we are working on reproducing the crash and will reach back out with additional information once we have more details to share.

@scannillo
Copy link
Contributor

scannillo commented Aug 6, 2024

Hi all. We have replicated this crash and are working on a fix!

(DTMOBILES-967 for internal tracking)

@sadaf-behbahani
Copy link
Author

Hi @sadaf-behbahani - thank you for raising this to our attention. We are investigating it currently. Please try v6.23.0 for now.

Do you have any additional log or stack trace details on the crash?

Thank you for looking into it, here is the stack trace :

OS Version:         iOS 17.5.1
Exception Type:     EXC_BREAKPOINT 
Exception Subtype:  KERN_INVALID_ADDRESS


EXC_BREAKPOINT: BUG IN CLIENT OF LIBDISPATCH: dispatch_sync called on queue already owned by current thread

0  libdispatch.dylib +0x13628 ___DISPATCH_WAIT_FOR_QUEUE__
1  libdispatch.dylib +0x13158 __dispatch_sync_f_slow
2  TouchTunes +0x69b9b0       ConfigurationCallbackStorage.add(_:) (ConfigurationCallbackStorage.swift:16:15)
3  TouchTunes +0x679d80       specialized ConfigurationLoader.getConfig(completion:) (<compiler-generated>)
4  TouchTunes +0x69dfc4       closure #1 in BTApplePayClient.tokenize(_:completion:) (BTApplePayClient.swift:94:28)
5  TouchTunes +0x6788fc       closure #1 in BTAPIClient.fetchOrReturnRemoteConfiguration(_:) (BTAPIClient.swift)
6  TouchTunes +0x69a5c4       thunk for @escaping @callee_guaranteed (@guaranteed BTConfiguration?, @guaranteed Error?) -> () (<compiler-generated>)
7  TouchTunes +0x69a4d8       closure #1 in closure #1 in ConfigurationCallbackStorage.invoke(_:_:) (ConfigurationCallbackStorage.swift:22:42)
8  TouchTunes +0x69a8c4       thunk for @callee_guaranteed () -> () (<compiler-generated>)
9  TouchTunes +0x69a8e4       thunk for @escaping @callee_guaranteed () -> () (<compiler-generated>)
10 libdispatch.dylib +0x3dd0  __dispatch_client_callout
11 libdispatch.dylib +0x132c0 __dispatch_lane_barrier_sync_invoke_and_complete
12 TouchTunes +0x69a404       ConfigurationCallbackStorage.invoke(_:_:) (ConfigurationCallbackStorage.swift:21:15)
13 TouchTunes +0x69ace0       ConfigurationLoader.notifyCompletions(_:_:) (ConfigurationLoader.swift:88:28)
14 TouchTunes +0x68d584       closure #1 in BTHTTP.callCompletionAsync(with:body:response:error:) (BTHTTP.swift:307:13)
15 TouchTunes +0x68a0c8       thunk for @escaping @callee_guaranteed @Sendable () -> () (<compiler-generated>)
16 libdispatch.dylib +0x2138  __dispatch_call_block_and_release
17 libdispatch.dylib +0x3dd0  __dispatch_client_callout
18 libdispatch.dylib +0x125a0 __dispatch_main_queue_drain
19 libdispatch.dylib +0x121b4 __dispatch_main_queue_callback_4CF
20 CoreFoundation +0x5670c    ___CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__
21 CoreFoundation +0x53910    ___CFRunLoopRun
22 CoreFoundation +0x52cd4    _CFRunLoopRunSpecific
23 GraphicsServices +0x11a4   _GSEventRunModal
24 UIKitCore +0x40a908        -[UIApplication _run]
25 UIKitCore +0x4be9cc        _UIApplicationMain
26 SwiftUI +0x3f4144          0x18c42c144 (0x18c42c0a0 + 164)
27 SwiftUI +0x3a0710          0x18c3d8710 (0x18c3d867c + 148)
28 SwiftUI +0x3ac4cc          0x18c3e44cc (0x18c3e444c + 128)
29 TouchTunes +0xc0d80        static AppLauncher.main() (AppLauncher.swift)
30 dyld +0x3ce48              start


@jaxdesmarais
Copy link
Contributor

Hey folks -

Thanks for your patience. We have an update up in the branch fix-config-callback-crash and are going through some final testing on our side but thus far are seeing the crash resolved. If y'all want to check on your end as well and let us know the results, please feel free, otherwise we will let you know once this has been released.

@dane-thomas-vs
Copy link

Hey folks -

Thanks for your patience. We have an update up in the branch fix-config-callback-crash and are going through some final testing on our side but thus far are seeing the crash resolved. If y'all want to check on your end as well and let us know the results, please feel free, otherwise we will let you know once this has been released.

Following up here: I did some tests using the PR branch #1385 and I was not able to reproduce the crash when simulating the memory warning. Thank you for your quick action on this issue! Looking forward to the official release. 🎉

jaxdesmarais added a commit that referenced this issue Aug 9, 2024
* Fixes Crash in ConfigurationCallbackStorage After Upgrading to 6.23.2 #1382
* Remove ConfigurationCallbackStorage
* Add async/throws get method
* Update getConfig() to use async/throws - use task block
* Update tests to use async/await methods

Signed-off-by: Jax DesMarais-Leder <[email protected]>
Co-authored-by: Jax DesMarais-Leder <[email protected]>
Co-authored-by: Victoria Park <[email protected]>
@jaxdesmarais jaxdesmarais reopened this Aug 9, 2024
@jaxdesmarais
Copy link
Contributor

The fix for this has been released in version 6.23.3. Please feel free to reopen this issue if you continue to see errors. Thanks again for your patience and reporting this issue to us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants