3

I'm asynchronously retrieving some rss articles with my Portable Class Library that uses the Microsoft.Bcl library (which doesn't have Task.WhenAll). Each article has a url to rss comments that I need to asynchronously retrieve as well.

The code below is my library. I call GetArticles() but it does not return any of the which creates a list of tasks that call GetComments() to asynchronously get the comments.

I've tried using Task.WaitAll in GetArticles to wait for the comments but it does not block the thread. Any help would be appreciated.

    private const string ArticlesUri = "";

    public async Task<List<ArticleBrief>> GetArticles()
    {
        var results = new List<ArticleBrief>();
        try
        {
            var wfw = XNamespace.Get("http://wellformedweb.org/CommentAPI/");
            var media = XNamespace.Get("http://search.yahoo.com/mrss/");
            var dc = XNamespace.Get("http://purl.org/dc/elements/1.1/");

            var t = await WebHttpRequestAsync(ArticlesUri);
            StringReader stringReader = new StringReader(t);
            using (var xmlReader = System.Xml.XmlReader.Create(stringReader))
            {
                var doc = System.Xml.Linq.XDocument.Load(xmlReader);
                results = (from e in doc.Element("rss").Element("channel").Elements("item")
                           select
                               new ArticleBrief()
                               {
                                   Title = e.Element("title").Value,
                                   Description = e.Element("description").Value,
                                   Published = Convert.ToDateTime(e.Element("pubDate").Value),
                                   Url = e.Element("link").Value,
                                   CommentUri = e.Element(wfw + "commentRss").Value,
                                   ThumbnailUri = e.Element(media + "thumbnail").FirstAttribute.Value,
                                   Categories = GetCategoryElements(e.Elements("category")),
                                   Creator = e.Element(dc + "creator").Value
                               }).ToList();

            }

            var tasks = new Queue<Task>();
            foreach (var result in results)
            {
                tasks.Enqueue(
                    Task.Factory.StartNew(async ()=>
                        {
                            result.Comments = await GetComments(result.CommentUri);
                        }
                    ));
            }

            Task.WaitAll(tasks.ToArray());
        }
        catch (Exception ex)
        {
            // should do some other
            // logging here. for now pass off
            // exception to callback on UI
            throw ex;
        }

        return results;
    }

    public async Task<List<Comment>> GetComments(string uri)
    {
        var results = new List<Comment>();
        try
        {
            var wfw = XNamespace.Get("http://wellformedweb.org/CommentAPI/");
            var media = XNamespace.Get("http://search.yahoo.com/mrss/");
            var dc = XNamespace.Get("http://purl.org/dc/elements/1.1/");

            var t = await WebHttpRequestAsync(uri);
            StringReader stringReader = new StringReader(t);
            using (var xmlReader = System.Xml.XmlReader.Create(stringReader))
            {
                var doc = System.Xml.Linq.XDocument.Load(xmlReader);
                results = (from e in doc.Element("rss").Element("channel").Elements("item")
                           select
                               new Comment()
                               {
                                   Description = e.Element("description").Value,
                                   Published = Convert.ToDateTime(e.Element("pubDate").Value),
                                   Url = e.Element("link").Value,
                                   Creator = e.Element(dc + "creator").Value
                               }).ToList();
            }
        }
        catch (Exception ex)
        {
            // should do some other
            // logging here. for now pass off
            // exception to callback on UI
            throw ex;
        }

        return results;
    }

    private static async Task<string> WebHttpRequestAsync(string url)
    {
        //TODO: look into getting 
        var request = WebRequest.Create(url);
        request.Method = "GET";

        var response = await request.GetResponseAsync();
        return ReadStreamFromResponse(response);
    }

    private static string ReadStreamFromResponse(WebResponse response)
    {
        using (Stream responseStream = response.GetResponseStream())
        using (StreamReader sr = new StreamReader(responseStream))
        {
            string strContent = sr.ReadToEnd();
            return strContent;
        }
    }

    private List<string> GetCategoryElements(IEnumerable<XElement> categories)
    {
        var listOfCategories = new List<string>();

        foreach (var category in categories)
        {
            listOfCategories.Add(category.Value);
        }

        return listOfCategories;
    }

Updated Code from Solution, just added .UnWrap() on the Enqueue method:

            var tasks = new Queue<Task>();
            foreach (var result in results)
            {
                tasks.Enqueue(
                    Task.Factory.StartNew(async ()=>
                        {
                            result.Comments = await GetComments(result.CommentUri);
                        }
                    ).Unwrap());
            }

            Task.WaitAll(tasks.ToArray());
Bryan
  • 347
  • 4
  • 14
  • 4
    `Task.Factory.StartNew(async ()=>` Anytime you see that it should be a red flag. It means that you'll have a `Task>`. The outer task is just starting the inner task, so when the outer task completes the inner one will just be starting. You should really be using `Task.Run` here, as it will automatically unwrap it into a `Task` for you. If you cannot, then you'll need to explicitly `Unwrap` it yourself. – Servy May 21 '13 at 17:04
  • 5
    Stephen Toub has an [excellent blog post about the differences between `Task.Run` and `Task.Factory.StartNew` and why you should prefer `Task.Run` for `async` tasks](http://blogs.msdn.com/b/pfxteam/archive/2011/10/24/10229468.aspx). – Stephen Cleary May 21 '13 at 19:44
  • Thank you for the comments and that article was very good. – Bryan May 22 '13 at 00:08
  • any final solution with full source code about it ? – Kiquenet Jan 01 '14 at 12:26

2 Answers2

4

It is waiting appropriately. The problem is that you are creating a Task which creates another task (i.e. StartNew is returning a Task<Task> and you are only waiting on the outer Task which completes rather quickly (it completes before the inner Task is complete)).

The questions will be:

  • Do you really want that inner task?
    • If yes, then you can use Task.Unwrap to get a proxy task that represents the completion of both the inner and outer Task and use that to Wait on.
    • If no, then you could remove the use of async/await in StartNew so that there is not an inner task (I think this would be prefered, it's not clear why you need the inner task).
  • Do you really need to do a synchronous Wait on an asynchronous Task? Read some of Stephen Cleary's blog: http://blog.stephencleary.com/2012/02/async-unit-tests-part-1-wrong-way.html

As an aside, if you are not using C# 5, then watch out for closing over the foreach variable result See

Community
  • 1
  • 1
Matt Smith
  • 17,026
  • 7
  • 53
  • 103
  • `Do you really need to do a synchronous Wait on an asynchronous Task?` Well, if he doesn't, how should he go about changing it, given that he can't use `WhenAll`? Hint: You can use `ContinueWhenAll` to write your own `WhenAll` method. – Servy May 21 '13 at 17:12
2

In Microsoft.Bcl.Async we couldn't add any static methods to Task. However, you can find most of the methods on TaskEx, for example, TaskEx.WhenAll() does exist.

Immo Landwerth
  • 3,239
  • 21
  • 22