Base Class Collection Thread Safe Question

Question

I have the following base class:

public class ReportBase {
    protected List ReportDataTables { get;set; }
}

We have this report base being used on multiple report classes - such as this:

public class MyCustomReport : ReportBase {
    public MyCustomReport() {}
    public void GenerateReport(string filters) {
      ReportDataTables = new List();
      //etc etc etc
    }
}

We are using autofac to dynamically swap out these various reports as needed for generation.

 public class ReportService : IReportService
    {
        private readonly IEnumerable> _reportGeneratorServices;

        public ReportService(IEnumerable> reportGeneratorServices)
        {
            _reportGeneratorServices = reportGeneratorServices;
        }

        public TestMethod(){
          //Not thread safe when called inside a Parallel.ForEach
          var reportServiceLazyLoad = _reportGeneratorServices.FirstOrDefault(s => s.Metadata.Name == myMetaDataName);
        }
     }

the problem we are encountering is that when we run a parallel.foreach and call the "TestMethod" for the same report type with different filters the underlying ReportDataTables is populated with data from multiple reports instead of specifically for the one being called. I'm not sure how to segment it so that it only loads up data for that specific service, it seems that when its in a parallel for each its mixing in results from other threads or something.

Not sure if this makes sense, the above code isn't exact but generalized to give a general idea. i can clarify more if necessary - thanks for all your help.

More Details To Illustrate the Problem

the code below i believe is illustrating the issue i'm encountering. I'm trying to dynamically pull the service out of the collection but i need it to be a new instance, not a reference, the problem is that when this occurs inside a parallel.foreach its using the same reference and updating data inaccurately, if it was using a seperate isolated instance i wouldn't have data being mixed across - if this makes sense?

public class Test {

    private readonly IEnumerable> _reportGeneratorServices;

    public Test(IEnumerable> reportGeneratorServices)
        {
            _reportGeneratorServices = reportGeneratorServices;
        }


     public void CheckRefEquality(){
           var t1 = _reportGeneratorServices.FirstOrDefault(s => s.Metadata.Name == "MyReportService");

            var t2 = _reportGeneratorServices.FirstOrDefault(s => s.Metadata.Name == "MyReportService");

            var t3 = Object.ReferenceEquals(t1.Value, t2.Value);
           //t3 returns true - how can i forcibly create a new instance instead of pulling the same instance, this is what i believe is causing issues.

   }

}

Update per Erics Example

In your sample code below:

fooService.GitErDone("Bar", "filter_1");
fooService.GitErDone("Bar", "filter_2");
fooService.GitErDone("Baz", "filter_3");
fooService.GitErDone("Bar", "filter_4");
fooService.GitErDone("Baz", "filter_5");

you said the above creates two instances, one per registration

what i'm trying to do is essentially the equivilanet of this - assuming each of the calls below happen concurrently in a parallel.foreach:

fooService.GitErDone("ReportService1", "filter_1");
fooService.GitErDone("ReportService1", "filter_2");
fooService.GitErDone("ReportService1", "filter_3");
fooService.GitErDone("ReportService1", "filter_4");
fooService.GitErDone("ReportService1", "filter_5");

Where essentially every one of the calls above uses a unique instance of ReportService1 - the problem is that when its sharing the instance it appears the underlying base class collection "ReportDataTables" is getting mixed data from different filters.

This may be a better representation

Parallel.ForEach(filters, f => {

var report =  _reportsCollection.FirstOrDefault(s => s.Metadata.Name == "AlwaysTheSameReport");

// report should be a new instance, however when i run this in reality its always the same instance instead of a new one
report.GetErDone(f);

});

user_141172 · Answer

var t3 = Object.ReferenceEquals(t1.Value, t2.Value);
//t3 returns true - how can i forcibly create a new instance instead of pulling the same instance, this is what i believe is causing issues.

Your dependency injection factory is creating singletons, meaning one unique object instance per type.

Autofac supports a variety of scope strategies. Instance per Dependency Scope seems suitable to this scenario.

An alternative is to make the method GenerateReport(string filters) thread safe (e.g. by moving protected List ReportDataTables { get;set; } to a local variable of that method).

UPDATE

Based on the following test, injected instances work as expected when filtered using metadata, both single and multi-threaded:

using Autofac;

namespace AutoFacTest
{
    public interface IFoo
    {
        void DoWork(string filter);
    }
    public class Foo : IFoo
    {
        static int instanceCounter = 0;
        private int thisInstanceNumber;
        public Foo()
        {
            thisInstanceNumber = Interlocked.Increment(ref instanceCounter);
            Console.WriteLine($"Constructing Foo #{thisInstanceNumber}");
        }
        public void DoWork(string filter)
        {
            Console.WriteLine($"Doing work in instance {thisInstanceNumber} with filter {filter}.");
        }
    }
    public class FooMetadata
    {
        public string? Name { get; set; }
    }
    public class DoStuffWithFoo
    {
        readonly IEnumerable> _foos;
        public DoStuffWithFoo(IEnumerable> foos)
        {
            _foos = foos;
        }
        public void GitErDone(string metaName, string filter)
        {
            var foo = _foos.First(f => f.Metadata.Name == metaName);
            foo.Value.DoWork(filter);
        }
    }
    internal class Program
    {
        private static IContainer? Container { get; set; }
        static void Main(string[] args)
        {
            MultiThreaded();
            //SingleThreaded_WorksAsExpected();
        }

        static void MultiThreaded_WorksAsExpected()
        {
            var builder = new ContainerBuilder();
            builder.RegisterType();

            var filters = Enumerable
                .Range(0, 100)
                .Select(n => $"Filter_{n}")
                .ToArray();

            builder.RegisterType()
                .WithMetadata(m => m.For(am => am.Name, "Bar"))
                .As()
                ;
            builder.RegisterType()
                .WithMetadata(m => m.For(am => am.Name, "Baz"))
                .As()
                ;
            Container = builder.Build();

            var fooService = Container.Resolve();

            int counter = 0;
            var names = new string[] { "Bar", "Baz" };
            Parallel.ForEach(filters, f =>
            {
                fooService.GitErDone(names[Interlocked.Increment(ref counter) % 2], f);
            });
        }

        static void SingleThreaded_WorksAsExpected()
        {
            var builder = new ContainerBuilder();
            builder.RegisterType();

            builder.RegisterType()
                .WithMetadata(m => m.For(am => am.Name, "Bar"))
                .As()
                ;
            builder.RegisterType()
                .WithMetadata(m => m.For(am => am.Name, "Baz"))
                .As()
                ;
            Container = builder.Build();

            var fooService = Container.Resolve();

            // These construct exactly 2 instances of Foo, one for each registration.
            // Logging in the method DoWork() indicates the expected instance is used.
            fooService.GitErDone("Bar", "filter_1");
            fooService.GitErDone("Bar", "filter_2");
            fooService.GitErDone("Baz", "filter_3");
            fooService.GitErDone("Bar", "filter_4");
            fooService.GitErDone("Baz", "filter_5");
        }
    }
}

Base Class Collection Thread Safe Question

1 Answers