parallel linq: AsParallel().forAll() nulls some objects

Question

So, I've got a very weird situation here where it seems a forAll() plinq-query removes some of my custom objects and, to be honest, I have no clue why.

var myArticles = data.FilterCustomerArticles([]params]).ToList(); //always returns 201 articles

result.Articles = new List<ArticleMinimal>();

try
{
    myArticles.AsParallel().ForAll(article =>
                    {
                        result.Articles.Add(new ArticleMinimal()
                        {
                            ArticleNumber = article.ArticleNumber,
                            Description = article.Description,
                            IsMaterial = false,
                            Price = article.PortionPrice.HasValue ? article.PortionPrice.Value : decimal.Zero,
                            Quantity = 1,
                            ValidFrom = new DateTime(1900, 1, 1),
                            ValidTo = new DateTime(2222, 1, 1)
                        });
                    });

}
catch (Exception ex)
{
    ...
}

The Code above returns different result counts nearly every time I call it. It should return 201 ArticleMinimal-Objects. Instead, it returns 200, 189, 19x... and from time to time 201, though. There happens no Exception, nothing. It just returns less objects than it should.

After changing the code to "good ol'" classy foreach-loop, I always get the expected 201 Objects.

Working Code:

var myArticles = data.FilterCustomerArticles([]params]).ToList(); //always returns 201 articles

result.Articles = new List<ArticleMinimal>();

try
{
    foreach (var article in myArticles) { 
        result.Articles.Add(new ArticleMinimal()
                        {
                            ArticleNumber = article.ArticleNumber,
                            Description = article.Description,
                            IsMaterial = false,
                            Price = article.PortionPrice.HasValue ? article.PortionPrice.Value : decimal.Zero,
                            Quantity = 1,
                            ValidFrom = new DateTime(1900, 1, 1),
                            ValidTo = new DateTime(2222, 1, 1)
                        });
    }

}
catch (Exception ex)
{
    ...
}

Additionally, after some more Lines of code, I have another forAll like this:

try
{
    result.Articles.AsParallel().ForAll(article =>
                {
                    if (article.Weight != null){
                        ...
                    }
                });
}
catch (Exception)
{
    ...
}

Using the first forAll, this throws a NullReferenceException - imho, cause it expects some 201 Objects, but some Listentries are null.

Now my actual Question is: Why is it, that the first forAll returns less objects than it should?! Only clue I could think of is the inline declaration of new ArticleMinimal(){ ...}); - but even if that's the cause it seems weird to me. Is it not possible to do this while using plinq? I'm just guessing here.

Hope you could help.

Best regards, Dom

You're manipulating a shared object (the `result.Articles` collection) from multiple threads, if this object is not thread-safe this will likely corrupt the object. — Lasse V. Karlsen, Jan 08 '16 at 10:11
Now, having commented that, and left my own answer, do you really need to do this in parallel? 201 articles, and thus 201 simple object constructions should not take much time at all, why complicate the matter? Personally I would just use the code from my answer and remove `.AsParallel()` and leave the rest as is until you think this is a performance problem. There could be things happening in the constructor of `ArticleMinimal` that takes time but then perhaps the name `Minimal` is incorrect? — Lasse V. Karlsen, Jan 08 '16 at 10:20
Thanks for your answer! The 201 articles are just my dev-test data. Talking about higher numbers for the actual live system. But valid question :) — Dominik, Jan 08 '16 at 10:21
OK, there is no way for anyone here to say that you should or should not use parallelism here so if you're aware of the difference and the possibilities, that's the only thing that matters. — Lasse V. Karlsen, Jan 08 '16 at 10:22

score 9 · Accepted Answer · answered Jan 08 '16 at 10:15

You cannot manipulate result.Articles from many threads as this will likely corrupt the internals, as you are observing.

Instead turn your parallel workflow into a pipeline that returns the created objects:

result.Articles.AddRange(myArticles.AsParallel().Select(article =>
    new ArticleMinimal()
    {
        ArticleNumber = article.ArticleNumber,
        Description = article.Description,
        IsMaterial = false,
        Price = article.PortionPrice.HasValue ? article.PortionPrice.Value : decimal.Zero,
        Quantity = 1,
        ValidFrom = new DateTime(1900, 1, 1),
        ValidTo = new DateTime(2222, 1, 1)
    })
);

The .Select here, since it is being executed on ParallelQuery returned by .AsParallel() will run in parallel on the items.

The .AddRange however will ask for ParallelQuery.GetEnumerator() which will return the items collected into one long collection, giving you what you want.

The fundamental difference is that .AddRange() will likely not add anything until all the parallel tasks have started to complete, whereas your way, if you add the appropriate locking, will add items to the collection as they are being produced. However, unless you want to observe items flowing into the collection as they are being produced this is unlikely to mean anything in your case.

Also see [@SchlaWiener](http://stackoverflow.com/users/98491/schlawiener)'s answer for an alternative way if you need it. — Lasse V. Karlsen, Jan 08 '16 at 10:23

score 5 · Answer 2 · edited May 23 '17 at 12:26

5

List.Add is not thread safe. Please refer to https://stackoverflow.com/a/8796528/98491

Use either lock

lock (result.Articles)
{
    result.Articles.Add(...);
}

or a thread safe collection. I would use a temporary collection and at the end use result.Articles.AddRange(...)

edited May 23 '17 at 12:26

Community

1
1

answered Jan 08 '16 at 10:16

Jürgen Steinblock

30,746
24
119
189

parallel linq: AsParallel().forAll() nulls some objects

2 Answers2