Ori Refael
Ori Refael

Reputation: 3018

c# mongodb possible deadlock in FindAsync extention

Im trying to cut out the code im writing. There is this piece of code where in every FindAsync we need to write:

using (var cursor = await SomeCollection.FindAsync(filter, options))
{
     while (await cursor.MoveNextAsync())
     {
          var batch = cursor.Current;
          foreach (var item in batch)
          {
              //do something
          }
     }
}

I came up with an extention. My code:

public static async Task<List<T>> GetResultsFromFindAsync<T>(this Task<IAsyncCursor<T>> find)
{
    List<T> result = new List<T>();
    using (var cursor = await find)
    {
        while (await cursor.MoveNextAsync())
        {
            var batch = cursor.Current;
            foreach (var item in batch)
            {
                result.Add(item);
            }
        }
    }

    return result;
}

and now, I only use:

List<MyObject> lst = await SomeCollection.FindAsync(filter, options).GetResultsFromFindAsync();

Question is: Can it cause deadlocks since there are 2 async tasks involved in a process with a single Await caluse. I know that the process really contains 2 Awaitcaluses, but, wont they just conflict which each other, or might cause to a data loss?

I executed a FindAsync using this extention and got my data, so its working, but the test doesn't gurentee deadlocks 100% of the times if it can occur.

I would very much like to know why or why not. Thanks.

Upvotes: 0

Views: 775

Answers (2)

Craig Wilson
Craig Wilson

Reputation: 12624

The .NET driver already offers extension methods for this type of thing. ToList, ToListAsync, and ForEachAsync are all available.

http://mongodb.github.io/mongo-csharp-driver/2.2/reference/driver/crud/reading/#iteration

Upvotes: 1

Kirill Shlenskiy
Kirill Shlenskiy

Reputation: 9587

Can this cause deadlocks? Yes, but not for the reasons you listed.

If someone decides to call your method synchronously and uses Wait or Result on the returned Task, they may get a deadlock. SO is littered with "why does my task never complete" questions for that exact reason. Use ConfigureAwait(false) on all Tasks awaited inside your GetResultsFromFindAsync to safeguard yourself.

With that said, if GetResultsFromFindAsync is always consumed asynchronously, there is no issue.

With regards to there only being a "single await clause" - that's not actually true. You are awaiting the Task returned by FindAsync inside your GetResultsFromFindAsync implementation, thus propagating its completion and exceptions (if any). When you call

SomeCollection.FindAsync(filter, options).GetResultsFromFindAsync()

, you are in effect awaiting both Tasks (as the inner Task is awaited by the outer Task). If FindAsync happens to throw (asynchronously), the Task returned by GetResultsFromFindAsync will automatically transition to Faulted state and the exception will be re-thrown when the outer Task is awaited.

In conclusion, there is nothing technically wrong with the method you have written, although introducing ConfigureAwait(false) wouldn't hurt.

EDIT

Having said all of the above, I would personally consider merging the two calls into one, so that calling FindAsync is the responsibility of GetResultsFromFindAsync. This is to prevent consumers from reusing the cursor returned by FindAsync as I expect the following will fail:

IAsyncCursor<T> cursor = await SomeCollection.FindAsync(filter, options);
List<MyObject> lst1 = await cursor.GetResultsFromFindAsync();
List<MyObject> lst2 = await cursor.GetResultsFromFindAsync(); // BOOM.

Upvotes: 2

Related Questions