2

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);
    }
}
Truning
  • 107
  • 1
  • 10
  • 1
    How are you creating a `Logger` instance? – Pavel Anikhouski Oct 30 '19 at 12:42
  • Is passing a path to the `Log` method as a parameter an option? Having a "stateful" class in a parallel environment is a pain. – max Oct 30 '19 at 12:57
  • I edited the post so you can see my implementations for the Logger class and JobProcessor. – Truning Oct 30 '19 at 13:07
  • @max It is an option, but I get to the same problem as before. Namely passing the path down from the facade to every other class. – Truning Oct 30 '19 at 13:09
  • Can't say that I've ever used the [`ThreadStaticAttribute`](https://learn.microsoft.com/en-us/dotnet/api/system.threadstaticattribute), but it seems like it might be what you're looking for? Perhaps marking `Logger._logger` with that attribute might cover it? – Joshua Robinson Oct 30 '19 at 13:50
  • @JoshuaRobinson This seems like EXACTLY what I have been looking for. I will see if I get it working. Many thanks in advance! – Truning Oct 30 '19 at 14:19
  • @JoshuaRobinson It works! Do you want to add an answer so I can mark it as correct? Or should I add it myself? Many thanks! – Truning Oct 31 '19 at 09:44
  • Great! Glad it helped @Truning. – Joshua Robinson Oct 31 '19 at 13:23

2 Answers2

1

You are able to create sort of Dictionary<ThreadId, BaseLogger> in your static logger class. You will have own logger for each thread.

Additionally, you can change SetLogger signature to smth like void SetLogger(Func<BaseLogger> loggerFactory) to create amount of loggers that you need inside of Logger.

  • Nice solution, but it seems equivalent to Joshuas answer about ThreadStaticAttribute with more work to implement. – Truning Oct 31 '19 at 09:47
1

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.

Joshua Robinson
  • 3,399
  • 7
  • 22