1

Closed

WinRtCredentials synchronization bugs

description

There are some serious synchronization bugs in the WinRtCredentials class. Both the LoadCredentialsFromStorageFile and SaveCredentialsToStorageFileAsync are async methods but they are called and used as synchronous.

The sample has the following code:
            var auth = new WinRtAuthorizer
            {
                Credentials = new LocalDataCredentials
                {
                    ConsumerKey = "",
                    ConsumerSecret = ""
                },
                UseCompression = true,
                Callback = new Uri("http://linqtotwitter.codeplex.com/")
            };

            if (auth == null || !auth.IsAuthorized)
            {
                await auth.AuthorizeAsync();
            }
The LocalDataCredentials constructor calls the WinRtCredentials constructor which is:
    public WinRtCredentials(StorageFolder folder)
    {
        this.folder = folder;
        LoadCredentialsFromStorageFile();
    }
The LoaCredentialsFromStorageFile starts as:
    async void LoadCredentialsFromStorageFile()
    {
        var credentialsFile =
            await folder.CreateFileAsync(
                "Linq2TwitterCredentials.txt",
                CreationCollisionOption.OpenIfExists);
The LoadCredentialsFromStorageFile is called in the constructor of the WinRtCrendentials which is called from the constructor of then LocalDataCredentials. Then it hits the await call to the CreateFileAsync and returns immediately BEFORE it loads any data. After that the constructors complete and control returns to the main program. So If the call to the auth.IsAuthorized is too close to the creation of the object (like in this example) it will return false, because the data are loading in another thread that it may not even have a chance to run yet.

The same problem exists in the property get methods. If the credentials[…] is null they call the LadCredentialsFromStorageFile to retrieve the data. But then they immediately continue to return a value that has not been loaded yet.

Using the await at the places that the LoadCredentialsFromStorageFile is called would not help either.

The best solution, I think, is to remove all the implicit calls of the LoadCredentialsFromStorageFile and make the LoadCredentialsFromStorageFile method public and explicitly called, so the user can call it at the correct time and using the await keyword. Another approach would be to use the Task.Wait method to block the thread until the data are loaded, but that would be considered a very bad practice in Windows 8.

Also I would like if the file name was given as a parameter ;)

There is another, minor, problem in the example: If the auth is null (auth == null) then you cannot call auth.AuthorizeAync. I suppose what you meant was: if( auth!=null && !auth.IsAuthorized) ….


P.S. The async and await keywords are very helpful but so easy to use that they can lead to some very nasty bugs
Closed Mar 12, 2013 at 7:24 AM by JoeMayo
Thanks for identifying this problem. I fixed the race conditions. I think that the way I handled the issue won't break any existing code either (should be rare if any).

Joe

comments