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

DataStore blocking main thread after toggling airplane mode on/off #2849

Closed
ethan021021 opened this issue Apr 4, 2023 · 12 comments
Closed
Assignees
Labels
bug Something isn't working datastore Issues related to the DataStore category requires attention Follow up needed for more than 10 days

Comments

@ethan021021
Copy link

ethan021021 commented Apr 4, 2023

Describe the bug

Hi! We are noticing a pretty serious issue with datastore when toggling airplane mode on/off then executing queries/saves. We can also reproduce this issue when general network loss issues occur as well. I believe it's blocking the main thread because when I try to call a completion handler and dispatch it's work back to the main thread the completion handler never ends up calling the code in the DispatchQueue.main.async {} block while datastore is stuck in this state. We've also noticed these other issues while datastore is in this state:

  • Any NSURLSession requests are blocked by datastore (blocked as in the request does not complete.. could maybe have to do with timeouts / datastore plugin taking over the default NSURLSession?)
  • Amplify plugin calls such as this one on Storage do not complete:
Amplify.Storage.getURL(key: awsKey) { [weak self] result in
            switch result {
            case .failure(let error):
                errorLog(function: #function, "Image loading error: \(error) key: \(awsKey)")
            case .success(let url):
                self?.loadImage(url: url)
            }
        }
  • Even after the phone reconnects to a stable connection queries/saves seem to be ignored..Maybe datastore is not restarting on reconnect?
  • Also noticed if you background the app, wait 5 seconds, then foreground the app again the app UI freezes for a bit then the app crashes.

This is a super frustrating bug for us as we are experiencing a ton of crashes.

Here's some GitHub issues that might have to do with this issue:
#1865
#2152

I can provide any additional context if needed. Thanks!

There is also a discord thread that I've created:

https://canary.discord.com/channels/705853757799399426/1092859891762348063

Steps To Reproduce

Steps to reproduce the behavior:
1. Run the application
2. Toggle airplane mode on
3. Wait 3 seconds
4. Turn airplane mode off
5. Try to make a query/save using datastore

Expected behavior

Main thread to not be blocked and network calls to complete

Amplify Framework Version

1.29.1

Amplify Categories

API, DataStore, Storage

Dependency manager

Swift PM

Swift version

5.5

CLI version

11.0.3

Xcode version

14.3

Relevant log output

N/A

Is this a regression?

Yes

Regression additional context

N/A

Device

iPhone 14 Pro

iOS Version

iOS 16.4

Specific to simulators

No response

Additional context

No response

@ethan021021
Copy link
Author

After some more investigation it might have to do with this:

Looks like there are about 40-50 threads trying to call AWSAuthService.getToken() and waits on the semaphore.

Screenshot 2023-04-04 at 12 54 45 PM

@ethan021021
Copy link
Author

Replacing semaphore.wait() on my own amplify fork with:

        if semaphore.wait(timeout: .now() + 2) == .timedOut {
            result = .failure(.unknown("Request was not completed in 2 seconds"))
        }

Seems to be a good workaround for now. I know the right way here is to use getUserPoolAccessToken() as getToken() is deprecated but I don't want to rewrite makeAPIRequest in IncomingAsyncSubscriptionEventPublisher.

@ethan021021
Copy link
Author

After some deep research this comment:

#1418 (comment)

Seems to be exactly what is happening but with getToken()

@royjit
Copy link
Contributor

royjit commented Apr 5, 2023

Thank you for reaching out and for the detailed analysis. We will investigate this further and update here.

@royjit royjit added bug Something isn't working datastore Issues related to the DataStore category requires attention Follow up needed for more than 10 days labels Apr 5, 2023
@ethan021021
Copy link
Author

@royjit Thanks Jithin! This is a serious problem for our app currently as we are experiencing many crashes/UI freezing from the system running up too many threads. Any chance the Amplify team can prioritize this and release a quick hot fix? Thank you!

@royjit
Copy link
Contributor

royjit commented Apr 7, 2023

Thank you for being patient here, we are actively working on the fix. Here is a draft PR I am working on: #2856. There are few different places where we use the synchronous getToken api and I am planning to create separate PRs for those. Will keep you updated here on the progress.

@ethan021021
Copy link
Author

Thanks @royjit ! Much appreciated

@royjit
Copy link
Contributor

royjit commented Apr 8, 2023

Hi @ethan021021 I was able to verify that the fix works via test, but I still could not reproduce the issue that you are seeing in an actual app. How big is your schema? How many models are present in the schema? Do you perform a lot of operation on Datastore while in airplane mode and when back online?

Will you be able to try the branch royjit.fixasyncgetToken in your testing and see if that fixes your issue?

@ethan021021
Copy link
Author

ethan021021 commented Apr 10, 2023

Hey @royjit, things are working great so far! Thanks a bunch for the quick fix. Looking forward to adding the work the other PR's contain so we can make the other calls to getToken async! Where are the other places getToken gets used? And does it affect DataStore/API? Thanks!

@royjit
Copy link
Contributor

royjit commented Apr 14, 2023

This PR has been released in the latest version of Amplify v1. We are currently working on fixing other areas that might have synchronous apis. As part of it we had to make few adjustment in the aws appsync realtime client. PRs for those changes are here:

aws-amplify/aws-appsync-realtime-client-ios#118
aws-amplify/aws-appsync-realtime-client-ios#119

We will then need to follow up on changes in Amplify V1.

Feel free to use the latest version of Amplify v1 and let us know if the fix works. Also it will help us if you can report back with any newer crash report with Amplify version 1.29.2.

@royjit
Copy link
Contributor

royjit commented Apr 20, 2023

Changes to Appsync Realtime Client are released as part of 3.1.0. Corresponding PRs for amplify-swift v1:

#2870 - Add async protocol implementation for token request interceptors
#2871 - Add async protocol implementation for IAM signing interceptors
#2876 - Add async implementation for URL request interceptors

@royjit royjit self-assigned this Apr 20, 2023
@ethan021021
Copy link
Author

This fixed it! Thank you! @royjit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working datastore Issues related to the DataStore category requires attention Follow up needed for more than 10 days
Projects
None yet
Development

No branches or pull requests

2 participants