0

I switched from a regular list to a ConcurrentBag for thread safety because the list was causing some clashes. The list is stored in cache and updated if a new item is found it is added to the list which has a reference to the cache list. I recently had a hang at the FirstOrDefault (regular LINQ, not database) code below on it. I am not sure why it would hang here. This is a high performance site and this list is hit regularly, is storing in the cache and using ConcurrentBag a good option for what I am doing?

Calling Code

var mobileApplicationsCached = GetMobileApplicationsCached();

var mobileApplicationCache = new AppDownloadModel();

if (mobileApplicationsCached != null)
{
    mobileApplicationCache = mobileApplicationsCached
        .FirstOrDefault(t => t != null && t.EventId == eventId ||
            (t.EventIds != null && t.EventIds.Contains(eventId)));

Get Cache List

public ConcurrentBag<AppDownloadModel> GetMobileApplicationsCached()
{
    return CacheHelper.GetInitializedCache(Config.Cache.Keys.MobileApplications, () =>
    {
        return new CacheData<ConcurrentBag<AppDownloadModel>>
        {
            Data = new ConcurrentBag<AppDownloadModel>()
        };
    }, Config.Cache.FourHours, false);
}

Dump Analysis

000000a3`24c096d0 00007ffc`37508b02     : 0000008f`03cb2ec8 00007ffc`8d3709f5 000000a3`24c09710 0000008f`8b566f98 : mscorlib!System.Collections.Generic.List<int>.Contains+0x38
000000a3`24c09720 00007ffc`3248e17a     : 0000008f`8b566e50 0000008f`03cb2ec8 00000000`00000000 00000000`00000000 : Tournaments_Services!Tournaments.Services.Mobile.MobileApplicationsService.<>c__DisplayClass5_0.<GetEventMobileApplication>b__0+0x82
000000a3`24c09770 00007ffc`374e42ed     : 0000008f`8b566ed0 000000a3`24c09870 00007ffc`8e53bd50 00007ffc`8e53e78f : System_Core!System.Linq.Enumerable.FirstOrDefault<Tournaments.Models.App.AppDownloadModel>+0xba
000000a3`24c097e0 00007ffc`3757902c     : 0000008f`8b55f2d8 00000000`0002a2ac 00000000`00006a00 00000000`00000000 : Tournaments_Services!Tournaments.Services.Mobile.MobileApplicationsService.GetEventMobileApplication+0x13d
000000a3`24c0a0d0 00007ffc`3795fd79     : 0000008f`8b561b88 00000000`0002a2ac 00006a00`24c0a301 0000008f`00000000 : Tournaments!Tournaments.Controllers.BaseEventController.AppleApplicationId+0xac
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Mike Flynn
  • 22,342
  • 54
  • 182
  • 341
  • What about the `AppDownloadModel` type? Is it mutable or immutable? If it's mutable, is it thread-safe? – Theodor Zoulias Mar 17 '22 at 12:55
  • It's mutable, and just a regular object. It's not modified after the creation and only read. – Mike Flynn Mar 17 '22 at 12:56
  • If it is only read after creation, why not use a regular list? Concurrent reads is perfectly thread safe. It is only concurrent writes, or writes and reads that are unsafe. – JonasH Mar 17 '22 at 12:57
  • Could you try switching from `ConcurrentBag` to `ConcurrentQueue`, and see if the hanging problem persists? The `ConcurrentQueue` has similar capabilities, and also preserves the insertion order. Instead of `Add` use `Enqueue`. – Theodor Zoulias Mar 17 '22 at 13:02
  • Well the list itself, had writes all over the place, along with reads. The objects themselves in the list don't get modified after added. I don't care about the order, should I? – Mike Flynn Mar 17 '22 at 13:03

1 Answers1

2

According to the documentation of the ConcurrentBag<T> method:

All public and protected members of ConcurrentBag<T> are thread-safe and may be used concurrently from multiple threads. However, members accessed through one of the interfaces the ConcurrentBag<T> implements, including extension methods, are not guaranteed to be thread safe and may need to be synchronized by the caller.

The FirstOrDefault LINQ operator is an extension on the IEnumerable<T>, an interface that the ConcurrentBag<T> implements, so officially you are in the "undefined behavior" territory. That said, and knowing how the FirstOrDefault is implemented, I see no reason for it to cause your application to hang.

Regarding the question whether a ConcurrentBag<T> is a good option for what you are doing, I would say it's unlikely. The ConcurrentBag<T> is a very specialized collection, making it a bad option for anything that it's not a mixed producer-consumer scenario. And judging from the code that you posted, your scenario is not one of those. I would suggest to switch to a ConcurrentQueue<T>, which is a collection that supports enqueuing (adding) and enumerating concurrently, and also preserves the insertion order of the items it contains.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104