Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm using a StaTaskScheduler (TPL extension) to open some web browser windows simultaneously and navigate to different sites (using WaitN, but it might as well just be a WebBrowser).

In my example below I'm running 5 concurrent browsers. Works fine, but it locked the UI thread, so I wrapped my tasks in a BackgroundWorker. Seems like it does the job, but is it the correct way of handling the issue? Do I need to dispose the worker once I'm done? And finally, is lock (ret) the correct way of keeping ret thread safe?

StaTaskScheduler sta = new StaTaskScheduler(5);
BackgroundWorker bw = new BackgroundWorker();

bw.DoWork += (s, ea) =>
{
    List<string> ret = new List<string>();

    lock (ret)

    Task.WaitAll(Util.GetList().Select(row => Task.Factory.StartNew(() =>
    {

        IEnumerable<string> result = DoUiLockingWork(row.Item1, row.Item2);
        ret.AddRange(result);

    }, CancellationToken.None, TaskCreationOptions.None, sta)).ToArray());

    ea.Result = ret;
};

bw.RunWorkerCompleted += (s, ea) =>
{
    List<string> result = ea.Result as List<string>;

    if (result != null) 
        Debug.WriteLine(result.Count);
};

bw.RunWorkerAsync();
share|improve this question

2 Answers 2

up vote 6 down vote accepted

I would go with Task-based method (if you're using .Net 4.5) instead of the BackgroundWorker. That will sort out most of your problems.

Something in the vicinities of

async Task<List<T>> yourJobAsync(...)
{
    var results = await Task.Run ( () =>
    {
        //your job.
        return jobResults
    });
    return  yourJobAsyncResults
}
share|improve this answer
    
Thanks, I actually changed the approach to your suggestion once I noticed that backgroundworkers were obselete. Can this return a result from all the nested tasks, without using lock? –  Johan Jun 13 at 7:41
1  
Yes it can. You must have found out the answer by now but in general Task<T> would return a T. If you use IEnumrable<T>` instead of the List you should be able to use Yield() to return the task result. Check this Microsoft post about yield return and the example. blogs.msdn.com/b/pfxteam/archive/2008/07/23/8768673.aspx –  Mehrad Jun 17 at 23:33

I would make your lock a little more tight:

lock (ret)
{
    ret.AddRange(result);
}

But otherwise, looks like an all right implementation.

share|improve this answer
1  
Thanks for the input. I also replaced the backgroundworker implementation with a pure Task solution. –  Johan Jun 12 at 22:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.