Search code examples
c#parallel-processingsingletonfacade

C# Singleton-Pattern not working as expected after implementing parallel instead of concurrent processing


Disclaimer: I know about questions like Thread Safe C# Singleton Pattern, however these do not answer my question.

Let me state my problem by first describing my C# project: I have a class JobProcessor that accepts an object of the class JobTicket, which holds information about "what to do". The JobProcessor uses the Facade Pattern to coordinate other classes according to the Parameters in JobTicket, such as e.g. the path to a .log file about that very Job. For that I have a Singleton class Logger, for which the JobProcessor set the path at the start of the Job and then every other class would just call Logger.Log(message). This worked fine until I got to the point to implement parallelization with a class JobManager that accepts objects of the class JobTicket and saves them in a queue. The JobManager instantiates a new JobProcessor with a JobTicket from its queue. And that with up to 4 in parallel.

Now you can already imagine what happened: If more than one JobProcessor runs at once, one overwrites the path of the Logger. How can I make sure that the Logger is contained inside the JobProcessor without changing a lot of code? I thought about having a Logger field in the JobProcessor class, but then I have to pass it down to every other class that wants to use it, since by the Facade pattern the facade knows every class it uses, but every class that is used by it does not know the facade.

In hindsight I see that the singleton pattern for the logger was not as smart as thought, but passing it down to every class seems tedious. Is there a smarter way? Can I have one singleton per thread?

public static class JobManager
{
    private static BlockingCollection<JobTicket> _jobs = new BlockingCollection<JobTicket>();

    public static void AddJob(JobTicket job)
    {
        _jobs.Add(job);
    }

    public static void StartConsumer()
    {
        Task.Factory.StartNew(() =>
        {
            void processJob()
            {
                while (!_jobs.IsCompleted)
                {
                    var job = _jobs.Take();
                    try
                    {
                        using (JobProcessor processor = new JobProcessor(job))
                        {
                            processor.Start();
                        }
                    }
                    catch
                    {
                        // Alert something that an error happened
                    }
                }
            };
            // process 4 jobs in parallel
            Parallel.Invoke(processJob, processJob, processJob, processJob); 
        });
    }
}

public class JobProcessor
{
    private JobTicket jobticket;

    public JobProcessor(JobTicket jobticket) {
        this.jobticket = jobticket;
        // ...
    }

    public void Start() {
        if(!(jobticket.LogPath is null)) {
            var logger = new TextLogger(jobticket.LogPath);
            Logger.SetLogger(logger);
        }
        Logger.Log("Job started");
        // Process job
        var a = new ClassA(...);
        if (jobticket.x)
            a.DoSomething();
        else
            a.DoSomethingElse();
    }
}

public class ClassA {
    //...
    public void DoSomething() {
        //...
        Logger.Log("I did something");
    }
    public void DoSomethingElse() {
        //...
        Logger.Log("I did something else");
    }
}

// I know this is not thread-safe, but what I want is one Logger instance per JobProcessor and not one Logger instance for all JobProcessors.
public static class Logger
{
    private static BaseLogger _logger = new ConsoleLogger();

    public static void Log(string message) {
        _logger.Log(message);
    }

    public static void SetLogger(BaseLogger logger)
    {
        _logger = logger;
    }
}

public abstract class BaseLogger
{
    public abstract void Log(string message);
}

public class TextLogger : BaseLogger
{
    public readonly string path;
    public TextLogger(string path) : base()
    {
        this.path = path;
    }

    public override void Log(string message)
    {
        File.AppendAllText(path, message);
    }
}

public class ConsoleLogger : BaseLogger
{
    public override void Log(string message)
    {
        Console.WriteLine(message);
    }
}

Solution

  • Based on the comments, it seems like ThreadStaticAttribute turned out to be the answer.

    According to the documentation for ThreadStaticAttribute:

    Indicates that the value of a static field is unique for each thread.

    So marking Logger._logger with that attribute should make each instance unique for each thread.