Calling async method from a loop

[alexteslin]

Hi,

I have a loop from which I call an async method that in turn calls several async methods. My loop example:

List<Task<ReturnItem>> savedItemTasks = new List<Task<ReturnItem>>();
foreach(var item in items)
{
    savedItemTasks.Add(SaveItem(item))
}

var result = await Task.WhenAll<ReturnItem>(savedItemTasks);

My SaveItem method example:

private async Task<ReturnItem> SaveItem(ItemType item)
{
    if(!IsValidItem(item))
        return;
    
    Guid? itemId = await GetItemId(item);
    
    if(itemId == null || itemId == Guid.Empty)
        return;
    
    var sourceId = await GetSourceId(item);
    
    if(sourceId == null || sourceId == Guid.Empty)
        sourceId = await CreateSourceId(item);
    
    // Some other async calls
    
    return new ReturnItem {
        ...
    };
}

The above loop works fine and in async. The problem is within the SaveItem() method when first checks for existing sourceId by calling GetSourceId() method and if it can't find then calls CreateSourceId() method. And the problem is that the CreateSourceId method depends on previous GetSourceId() method. Even though I am using await keywords these are called from the loop. And if the sourceId is null or empty guid on first iteration it creates correctly the sourceId by calling CreateSourceId() method. But if the second immediate item has no sourceId either it then calls again for CreateSourceId(), in which case it creates a duplicate.
What I am after is that to wait until the sourceId from the previous item has been created so that then I can use that value.
Is this possible at all? I don't want to make the loop synchronous as there are quite a number of items and there are more methods and saving to db etc, which will take much longer to run the code.

Thanks,
Alex

My

 

[Skydiver]

But if the second immediate item has no sourceId either it then calls again for CreateSourceId(), in which case it creates a duplicate.

Why would it create a duplicate? Your method seems to be misnamed if two consecutive calls create a duplicate. This is not an issue of calling an async method in a loop. You could unroll the loop and still have the same issue.

 

[alexteslin]

Why would it create a duplicate? Your method seems to be misnamed if two consecutive calls create a duplicate. This is not an issue of calling an async method in a loop. You could unroll the loop and still have the same issue.

I don't understand what you mean by my method seems to be misnamed. The issue is that I am not using await within the loop so that the methods are called almost the same time. So for example, when first item calls CreateSourceId(), the second iteration item returns null or empty guid from GetSourceId(), because it entered before the first completed to create new sourceId and tries to create again.

 

[JohnH]

the problem is that the CreateSourceId method depends on previous GetSourceId() method

Did you mean CreateSourceId again?

it then calls again for CreateSourceId(), in which case it creates a duplicate.

Doesn't it return a new Guid? (since you compare with Guid.Empty)

 

[alexteslin]

Did you mean CreateSourceId again?

Doesn't it return a new Guid? (since you compare with Guid.Empty)

I think I need to explain the business logic as there is a confusion. Let's say I have 2 items in the list in my foreach loop.
Item1.SourceName = "Microsoft"
item2.SourceName = "Microsoft"
So the source names for both items are the same. When item1 is passed to SaveItem() method it calls GetSourceId() method. It cannot find and it calls CreateSourceId() method. As the loop is not synchronous item2 is also passed to SaveItem() method and it calls GetSourceId() method. It cannot find it as the item1 has not returned from CreateSourceId() yet. Item2 then calls CreateSourceId() method as well to try to create a SourceId with the same name, as the SourceNames are the same for item1 and item2.
I hope I made it a bit clearer what is going on.
I thought of a workaround: before the loop process, to get all SourceNames that has no Ids and create SourceIds in synchronous way, store it in temp key:value collection and then process the loop and use the values from this collection, but I thought maybe there is a way to handle the duplicate issue within the current loop process.

 

[JohnH]

You could use the lock statement around the "linked" GetSourceId and CreateSourceId calls to make it an atomic "GetOrCreate" and ensure only one thread gets or creates the id at the same time.

 

[Skydiver]

As I recall there are restrictions for using lock along with async/await, but I'm recovering from the flu right now and my brain is still quite fuzzy.

 

[JohnH]

As I recall there are restrictions for using lock along with async/await, but I'm recovering from the flu right now and my brain is still quite fuzzy.

True, await operator - C# reference

Within an async method, you can't use the await operator in the body of a synchronous function, inside the block of a lock statement,

There are other thread syncing ways, for example SemaphoreSlim that has async waiting too.

 

[Skydiver]

Also note that lock locks out  another thread from the locked section. async/await does not create another thread or enqueue work in the thread pool. (It's the Task.Run() that does thread pool job queuing.)

So as noted above, you'll need to use some other synchronization mechanism like a semaphore or mutex.

 

[alexteslin]

Also note that lock locks out  another thread from the locked section. async/await does not create another thread or enqueue work in the thread pool. (It's the Task.Run() that does thread pool job queuing.)

So as noted above, you'll need to use some other synchronization mechanism like a semaphore or mutex.

Are you suggesting to use semaphore within my loop? Or only when calling these 2 methods, Get and Create?

 

[Skydiver]

Not within the loop. The loop is irrelevant here as I noted above. If you unroll the loop you still have the same issue. The issue is the two consecutive calls and your current architecture being unable to handle it.

You'll need some kind of synchronization around the get-and-create-if-not-present logic.

 

[alexteslin]

As suggested above I've used SemaphoreLight and it seems working now as expected:

private readonly SemaphoreSlim _lock = new SemaphoreSlim(1,1);

private async Task<ReturnItem> SaveItem(ItemType item)
{
    if(!IsValidItem(item))
        return;
    
    Guid? itemId = await GetItemId(item);
    
    if(itemId == null || itemId == Guid.Empty)
        return;
    
    await _lock.WaitAsync();
    var sourceId = Guid.Empty;
    try
    {
        sourceId = GetSourceId(item);
        
        if(sourceId == null || sourceId == Guid.Empty)
        sourceId = CreateSourceId(item);
    }
    finally
    {
        _lock.Release();
    } 
    
    // Some other async calls
    
    return new ReturnItem {
        ...
    };
}

I haven't tested the performance, but there is nothing can be done from that point anyway.

Thank you guys for your help, really appreciated.

 

[Skydiver]

If you are going to use synchronous calls:

sourceId = GetSourceId(item);
        
if(sourceId == null || sourceId == Guid.Empty)
    sourceId = CreateSourceId(item);

then you don't even need the lock around that bit of synchronous code because you'll never yield control to get back into the scenario you previously described (unless your SaveItem() might be called from multiple threads).

 

[alexteslin]

If you are going to use synchronous calls:

sourceId = GetSourceId(item);
       
if(sourceId == null || sourceId == Guid.Empty)
    sourceId = CreateSourceId(item);

then you don't even need the lock around that bit of synchronous code because you'll never yield control to get back into the scenario you previously described (unless your SaveItem() might be called from multiple threads).

Yes, you are right. That was the problem, SaveItem() method is called from multiple threads because my loop adds them into a Task list. So now by calling synchronous way these two methods with Semaphore worked.

 

[Skydiver]

Unless your code in post #1 is running in a console app, there are no multiple threads from the for loop.