CODEWITHSUNDEEP
c#async-awaittask-parallel-librarytask
Igor S
Igor S

Reputation: 638

TPL control flow issue

I have three classes:

BargeGrouper class which implements IBargeGrouper and has Group(IEnumerable<Barge> ungrouped) method.

BargeGroupMap class which implements IBargeGroupShow and has Show(IEnumerable<BargeGroup> method.

And the class where I call both if these into: GroupingModule and it's Run().

Problem is that when I call Group(ungrouped); and then want to Show(bargeGroups); I got IndexOutOfRangeexception in Show(bargeGroups); because theIEnumerable I pass to it as a parameter has Count property many more than actual elements in it. I found out that grouped collection is being returned as empty from Group most of the time despite I use ContinueWith to fill that collection with BargeGroup elements.

Group method of BargeGrouper

public IEnumerable<BargeGroup> Group(IEnumerable<Barge> ungroupedBarges)
{
    List<BargeGroup> bargeGroups = new List<BargeGroup>();
    Int32 groupNumber = 0;

    var riverBarges = from barge in ungroupedBarges
                      where barge != null && !String.IsNullOrEmpty(String.Intern(barge.River))
                      let river = String.Intern(barge.River)
                      orderby river, barge.MileMarker ascending
                      group barge by river into barges
                      select barges.AsEnumerable();

    foreach (IEnumerable<Barge> barges in riverBarges)
    {
        Task.Run(() => ResolveRiver(barges, ref groupNumber)).ContinueWith(t=>
        {
            IEnumerable<BargeGroup> riverGroups = t.Result;
            bargeGroups.AddRange(riverGroups);
        });
    }

    return bargeGroups;
}

ShowBargeGroups:

public void ShowBargeGroups(IEnumerable<BargeGroup> bargeGroups)
{
    Console.WriteLine("Barge groups :");
    if (bargeGroups != null)
    {
        foreach (var group in bargeGroups.Where(b => b != null))
        {
            var title = String.Format("{0}\n\t | {1} \t\t | {2} \t | {3} \t|", group.Id, "Id", "River", "MileMarker");
            Console.WriteLine(title);
            foreach (var barge in group.Barges)
            {
                var caption = String.Format("\t | {0}\t | {1} \t | {2} \t|", barge.Id, barge.River, barge.MileMarker);
                Console.WriteLine(caption);
            }
        }
    }
}

And usage in GroupingModule:.

var groupBarges = bargeGrouper.Group(barges);

bargeGroupShow.ShowBargeGroups(groupBarges);

What can I do to fix this?

Upvotes: 1

Views: 123

Answers (4)

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149558

When using Task.Run, which returns a Task without asynchronously waiting on it, execution of the next line will happen immediately, it won't wait for the delegate passed to complete.

One way of resolving this would be to issue the calls to ResolveRiver in parallel and asynchronously wait for multiple tasks to complete:

public Task<IEnumerable<BargeGroup>> GroupAsync(IEnumerable<Barge> ungroupedBarges)
{
    // Do the grouping

    var riverTasks = riverBarges.Select(barges => 
                                        Task.Run(ResolveRiver(barges, ref groupNumber)));

    var result = await Task.WhenAll(riverTasks);
    bargeGroups.AddRange(result.Result.SelectMany(x => x));
    return bargeGroups;
}

And consume it like this:

public async Task FooAsync()
{
    var barges = await GroupAsync(ungroupedBarges);
    ShowBargeGroups(barges);
}

Note I'd be careful with the ref parameter being passed and called in parallel, that won't be safe. If you can remove the groupNumber from the equation, do so.

Upvotes: 2

Can Bilgin
Can Bilgin

Reputation: 509

Task.Run in your code creates another awaitable which is most likely going to be executed after your function returns. The easiest solution would be to wait for it with the Wait function.

Task.Run(() => ResolveRiver(barges, ref groupNumber)).ContinueWith(t=>
    {
        IEnumerable<BargeGroup> riverGroups = t.Result;
        bargeGroups.AddRange(riverGroups);
    }).Wait()

or event better, you do not need the Task.Run, just waiting for the continuation function would be enough:

ResolveRiver(barges, ref groupNumber)).ContinueWith(t=>
    {
        IEnumerable<BargeGroup> riverGroups = t.Result;
        bargeGroups.AddRange(riverGroups);
    }).Wait();

Upvotes: 0

norekhov
norekhov

Reputation: 4318

Task.Run doesn't do it's job immediately. So at least you've got a race condition in your code. Try it like this:

public async Task<IEnumerable<BargeGroup>> Group(IEnumerable<Barge> ungroupedBarges)
{
    List<BargeGroup> bargeGroups = new List<BargeGroup>();
    Int32 groupNumber = 0;

    var riverBarges = from barge in ungroupedBarges
                      where barge != null && !String.IsNullOrEmpty(String.Intern(barge.River))
                      let river = String.Intern(barge.River)
                      orderby river, barge.MileMarker ascending
                      group barge by river into barges
                      select barges.AsEnumerable();

    foreach (IEnumerable<Barge> barges in riverBarges)
    {
        var riverGroups = await Task.Run(() => ResolveRiver(barges, ref groupNumber));
        bargeGroups.AddRange(riverGroups);
    }

    return bargeGroups;
}

Upvotes: 1

jnovo
jnovo

Reputation: 5779

There is no guarantee that by the time you return from the Group method the Task is done processing the bargeGroups list. I suspect that your ResolveRiver method might take some time.

You need to wait for the task doing the processing to end before returning the enumerable from Group. I'd suggest making Group async and awaiting for the Task:

public async Task<IEnumerable<BargeGroup>> Group(IEnumerable<Barge> ungroupedBarges)
{
    List<BargeGroup> bargeGroups = new List<BargeGroup>();
    Int32 groupNumber = 0;

    var riverBarges = from barge in ungroupedBarges
                      where barge != null && !String.IsNullOrEmpty(String.Intern(barge.River))
                      let river = String.Intern(barge.River)
                      orderby river, barge.MileMarker ascending
                      group barge by river into barges
                      select barges.AsEnumerable();

    foreach (IEnumerable<Barge> barges in riverBarges)
    {
        await Task.Run(() => ResolveRiver(barges, ref groupNumber)).ContinueWith(t=>
        {
            IEnumerable<BargeGroup> riverGroups = t.Result;
            bargeGroups.AddRange(riverGroups);
        });
    }

    return bargeGroups;
}

Indeed, the proper way to go would be to make ResolveRiver async and await it. Then you could get rid of the bargeGroups List that you are using to communicate with the Task.

If you don't want to go the async/await way you may just .Wait() the Task.

Upvotes: 0

Related Questions