amazon-archives / aws-sdk-unity

ARCHIVED: The aws sdk for unity is now distributed as a part of aws sdk for dotnet:
https://github.com/aws/aws-sdk-net
Other
105 stars 43 forks source link

unity sdk making synchronous calls internally #118

Closed madangosain closed 8 years ago

madangosain commented 8 years ago

AWS SDK 2.2, Unity 5.2.4 Max OS 10.10

1) Calling GetCognitoIdAsync it internally sometime refreshes the credentials synchronously which leads to dead lock. Logs attached. Is there any workaround?

If this error happen, it'll keep crashing the application forever even after restart. I've to hard delete all locally saved data(credentials) to make it work again. This is a blocker. It's okay if AWS SDK cannot be initialized in case of any error, then game can default to offline mode. But crashing/hanging the game is a blocker. Can we please get an update on this?

As a workaround, when does AWS refresh credentials ? If my game can detect it, then as a work around, my game can clean credentials before AWS try to refresh it.

2) Call GetItem or any first call to the table internally calls Table GetDescription synchronously which again hangs the game. Work-around: Call Table.LoadTableAsync everytime when game starts before making any other call to table.

Both of these problems hangs the editor at "WaitOne". What is the suggested workaround? This crashes the application

karthiksaligrama commented 8 years ago

You mean SDK 2.1?

You should make all calls to Synchronous API's in a background thread. For all purposes you should use Async API's (Please note that Synchronous API's are marked internal)

Also i'm not sure why your editor is hanging, are you not getting this exception?

Can you point me to the GetCognitoIdAsync api in the source code? Or do you mean GetIdentityIdAsync? The error messages that you received in the log all seem valid to me.

madangosain commented 8 years ago

aws-unity-sdk-2.1.0.1 2 aws-sdk-unity-dynamodbv2-2.1.0.1.unitypackage

You are correct, it's 2.1.
I am making calls to Asyncronous api only. I believe internally it makes call to sync api automatically when try to read data from cache or refresh credentials. It try to refresh credentials at random interval where it hangs the editor. When table description is not found in cache, it make sync call to get table description. I do not see main thread exception. I suspect there is another internal code path to make synchronous calls which may not have that check.

Simple way to reproduce is to Make a Query call to any table. Make sure it is your first call to the table in .Net object persistence style as: Note: Your email id is not listed on github, if you can provide that. I can mail you the files.

1)

    public CognitoAWSCredentials CognitoCredentials {
        get {
            if (_cognitoCredentials == null) {
                MogLog.Warn("Initializing anonymous login");
                _cognitoCredentials = new CognitoAWSCredentials(Mogas.Instance.IdentityPoolId, RegionEndpoint.USEast1);
            }
            return _cognitoCredentials;
        }
    }

    public AWSCredentials Credentials {
        get {
            return CognitoCredentials;
        }
    }

    public IAmazonDynamoDB DynamoDbClient {
        get {
            if (_ddbClient == null) {
                _ddbClient = new AmazonDynamoDBClient(Credentials, RegionEndpoint.USEast1);
            }
            return _ddbClient;
        }
    }

    void GetCognitoId(float delay=2F) {
        CognitoCredentials.GetIdentityIdAsync(delegate(AmazonCognitoIdentityResult<string> result) {
            if (result.Exception != null) {
                return;
            }
            CognitoId = result.Response;
            Debug.LogError("User Id: " + CognitoId);
            IsInitialized = true;
        });
    }

    private void SearchAll<T>(AsyncSearch<T> search, Action<List<T>> onGet=null) {
        List<T> queryResults = new List<T>();

        search.GetNextSetAsync((result)=> {
            if (result.Exception != null) {
            } else {
                queryResults = result.Result;
            }
        });
    }

1) Call GetCognitoId as above 2) Call SearchAll(Context.QueryAsync(primaryHashKey), onGet); 3) At the same time send several LoadAsync calls to other tables Make sure that this is the first call to table and the table description is not already loaded in sdk cache. Please check that internally it'll try to get table description which sometime hangs at waitone. It's more likely to go into deadlock, if you combine it with simultaneous loadasyn, saveasync calls interacting other tables (not the table we are querying on).

madangosain commented 8 years ago

I simply call: CognitoCredentials.GetIdentityIdAsync(delegate(AmazonCognitoIdentityResult result) {

where

    public CognitoAWSCredentials CognitoCredentials {
        get {
            if (_cognitoCredentials == null) {
                MogLog.Warn("Initializing anonymous login");
                _cognitoCredentials = new CognitoAWSCredentials(IdentityPoolId, RegionEndpoint.USEast1);
            }
            return _cognitoCredentials;
        }
    }

It hangs the editor at CredentialRetriever.cs

    public override IAsyncResult InvokeAsync(IAsyncExecutionContext executionContext)
    {
        PreInvoke(ExecutionContext.CreateFromAsyncContext(executionContext));
        return base.InvokeAsync(executionContext);
    }

This method is async but it internally calls this PreInvoke method

protected virtual void PreInvoke(IExecutionContext executionContext) { ImmutableCredentials ic = null; if (Credentials != null && !(Credentials is AnonymousAWSCredentials)) { using(executionContext.RequestContext.Metrics.StartEvent(Metric.CredentialsRequestTime)) { ic = Credentials.GetCredentials(); } } executionContext.RequestContext.ImmutableCredentials = ic; }

which makes a syncronous calls at ic = Credentials.GetCredentials(); - this hangs the editor when it internally makes a syncronous call to:

private GetCredentialsForIdentityResponse GetCredentialsForIdentity(GetCredentialsForIdentityRequest getCredentialsRequest) { var result = cib.GetCredentialsForIdentity(getCredentialsRequest); -- SYNC CALL return result; }

Attached is the stack trace aws=getidentity-hang.txt

In similar way, other async API makes sync calls when reading data from cache. If cache does not find the data eg: Table description - it makes a synchronous call.

karthiksaligrama commented 8 years ago

The SDK does make internal Synchronous calls and thats valid, but it shouldn't cause issues like you are facing.

I'll need to investigate some more. Is it possible for you to upload a sample to some github repo so that i can reproduce this issue? If not you can also send me the code privately @ saligram@amazon.com.

madangosain commented 8 years ago

It has third party plugin, so I cannot share the project publicly. But I've shared the project with you on bitbucket and also provided details in email from my "gmail" account - madangosain0@gmail.com

Thank you, please keep me posted.

karthiksaligrama commented 8 years ago

Got it.. i'll take a look at it now.

madangosain commented 8 years ago

Thank you Karthik,

Did you get time to look at it? This is currently the release blocker for us, so I'll appreciate any workaround.

karthiksaligrama commented 8 years ago

still looking at it. The wait handle seems to be blocking all the threads when its supposed to block only the thread on which its waiting i.e the background thread. Also seeing if unity made any recent changes in the way thread work.

madangosain commented 8 years ago

Does it use to work for older version of Unity? Which is the recommended/supported/tested version of Unity ?

karthiksaligrama commented 8 years ago

This is related to #88 . The following API need to be called on Background thread CreateBatchWrite, CreateBatchGet, QueryAsync, ScanAsync.

I'm not sure why you did not get the Network on mainthread exception. But i just tested your code seems to run when you calls these api's on the threadpool thread.

madangosain commented 8 years ago

Can you please explain how to call these api's on threadpool thread? I've noticed that even LoadAsync or SaveAsync sometime hangs the editor when trying to read table config from aws cache. For the same reason, I'm loading all tables when the game starts to avoid the problem. If you can please elaborate on your workaround, I'll be glad to use it.

But the bigger problem - Cognito getidentity async still exists? As I need cognito identity first to access dynamodb, lambda, s3 or other services. Cognito identity async works fine initially but it starts hanging the game if project is idle for 3-12hours. get Cognito identity async call refresh credentials after some inactivity period which hangs the editor. Does your workaround will work here also?

karthiksaligrama commented 8 years ago

You dont really have to get identity id, before making the calls to dynamodb, the sdk does that for you.

With regards to how to use thread pool its something like

ThreadPool.QueueUserWorkItem((state)=>{
     Context.CreateBatchWrite(...)
});
karthiksaligrama commented 8 years ago

I'll push out a fix for this in the next patch so you don't have to invoke the request inside of the thread pool

madangosain commented 8 years ago

I need the Id to control access on rows for dynamodb. I am using cognito id as primary key for my dynamo db.For the problem, my game gets stuck at initialization only. Logs are attached with the initial request. Will your patch fix issue described in this thread also - Cognito Refresh Credentials? User have no control over it as this happens initially after some inactivity.

karthiksaligrama commented 8 years ago

The project you sent me i 'm not able to reproduce the error for cognito identity. I successfully get the identity id.

madangosain commented 8 years ago

I can reproduce it everytime with facebook login. You still be able to reproduce it with anonymous login. 1) Login using anonymous, record the identity id 2) Play around for couple of minutes to ensure all calls are finished 3) Leave the project inactive for 5mins 4) Open and run the project again, login as anonymous. You should see the same identity id as before 5) Now Leave the project inactive for 12 hours 6) Open and run the project again, login as anonymous. Somehow AWS figure out that the token is old and try to refresh the token. It hangs the editor. Once you reach this stage, any restart will not help unless you forcefully clear unity playerprefs.

You can get exact method details from the logs attached with initial thread. I think your threadpool wrap around refresh method may work here as well. Please let me know if you are able to reproduce the problem successfully.

madangosain commented 8 years ago

aws.getidentity-hang.txt

Decavoid commented 8 years ago

aws.getidentity-hang.txt is a stacktrace of the ThreadPool thread. You need to get stacktrace of the main thread. When the editor hangs, launch MonoDevelop. Run - Attach to process, Attach. Run - Pause. View - Debug Windows - Threads. Double click on the first thread. Look at the callstack (View - Debug Windows - Call Stack).

madangosain commented 8 years ago

That is exactly what I did: When the editor hangs, launch MonoDevelop. Run - Attach to process, Attach. Run - Pause. View - Debug Windows - Threads.

I clicked on all threads. Only one of them has stack trace which is attached in aws.getidentity-hang.txt. Clicking on any other thread results in an error popup.

madangosain commented 8 years ago

Consistently happening again:

ThreadId 1 AWSSDK/src/Core/Amazon.Runtime/AWSCredentials.cs:705 There is no stack trace - mono error popup. Send error report popup

Thread Id 2 has attached stack trace as original stack

Decavoid commented 8 years ago

So, partial stack trace for the main thread is: AWSSDK/src/Core/Amazon.Runtime/AWSCredentials.cs:705 It looks like the main thread is waiting for ThreadPool thread to release RefreshingAWSCredentials._refreshLock, but ThreadPool thread is waiting for the main thread to set UnityWebRequest.WaitHandle. Deadlock.

madangosain commented 8 years ago

Issue is always reproducible. Just wait for token to expire. SDK try to refresh credentials automatically and block the game. Even restart does not help. I've to clear playerprefs if i ever have to play game again. I am not sure why it will work for some user as I am able to reproduce it in multiple projects repeatedly. What is the suggested workaround?

Decavoid commented 8 years ago

In order to make token expire sooner change AWSCredentials.cs:778 from var exp = _currentState.Expiration.ToUniversalTime(); to var exp = _currentState.Expiration.ToUniversalTime() - TimeSpan.FromMinutes(59.5); UnityEngine.Debug.Log(string.Format("now {0} exp {1} ShouldUpdate {2}", now, exp, now > exp));

madangosain commented 8 years ago

Thanks, Are you proposing a workaround? Or an easy way to reproduce the problem?

Decavoid commented 8 years ago

An easy way to reproduce the problem.

madangosain commented 8 years ago

hi @karthiksaligrama , Does your patch fix this as well? If yes, how do I get the patch unitypackage? If no, what is the suggested workaround?

madangosain commented 8 years ago

if (CognitoCredentials.ShouldUpdate) { CognitoCredentials.GetCredentials(); } This call alone blocks the game whenever execute from any file. I can call it directly after I've retrieved CognitoCredentials (exposing ShouldUpdate). This somehow creates a dead lock by iteself when ShouldUpdate is true (__currentState are null)

Decavoid commented 8 years ago

GetCredentials() must be called on a background thread, not the main thread.

madangosain commented 8 years ago

Yes, you cannot call it on main thread. You can call it on background thread and it'll hang the game. Internally it invokes something on main thread.

karthiksaligrama commented 8 years ago

Please try our latest sdk. If you continue to face problems then please open a new issue in aws sdk for dotnet repository