Circular dependency with cross-cutting concern

Question

So, this example is contrived to try to give a simple view of a much larger system I am trying to modify (namely, Orchard CMS). As such, it may not be perfect.

I am trying to create a logging system that is managed through settings. The problem I'm running into is that retrieving the settings causes logging to occur. Here's a simple example that hopefully describes the problem:

static void Main(string[] args)
{
    string[] messages = "this is a test.  but it's going to be an issue!".Split(' ');

    Parallel.ForEach(messages, Log);

    Console.ReadLine();
}

public static void Log(string message)
{
    Console.WriteLine(GetPrefix() + message);
}

public static string GetPrefix()
{
    Log("Getting prefix!");
    return "Prefix: ";
}

This is an obvious StackOverflowException. However, how can I resolve it? I can't just disable the logging until I get the response from GetPrefix, because I may miss logs. (In fact, in this simple example, I miss all but the first.)

static void Main(string[] args)
{
    string[] messages = "this is a test.  but it's going to be an issue!".Split(' ');

    Parallel.ForEach(messages, Log);

    Console.ReadLine();
}

static bool _disable = false;
public static void Log(string message)
{
    if (_disable)
    {
        return;
    }
    _disable = true;
    Console.WriteLine(GetPrefix() + message);
    _disable = false;
}

public static string GetPrefix()
{
    Log("Getting prefix!");
    return "Prefix: ";
}

(^Bad.)

Note that I do not currently have control over the GetPrefix method, only the Log method.

I'm not sure if there's a way to resolve this; I may need to put the settings elsewhere (such as the config or a separate settings file). However, if anyone has ideas or suggestions, I'd be happy to try anything, as I'd prefer to leave the settings as I have them now (which is in an admin interface).

You have to synchronize access to `bool _disable`. You lose the parallel execution, which is fine in this example because `Console.WriteLine` is synchronized anyway. — JosephHirn, Jul 18 '13 at 17:33
@Ginosaji: This is a simple example, but, in the real one, synchronizing will have a severe performance impact. — zimdanen, Jul 18 '13 at 17:34
You can still process the messages in parallel and just synchronize the logging. As I said, `Console.WriteLine` is synchronized anyway so you're losing nothing by locking `bool _disable`. — JosephHirn, Jul 18 '13 at 17:35
@Ginosaji: `Console.WriteLine` is not used in the real solution. Assume we can't log serially - it needs to be able to process many logs at a time. — zimdanen, Jul 18 '13 at 17:44

JayC · Answer 1 · 2013-07-18T19:10:49.480

All you need to do is to disable on the current stack frame. Now you could use reflection to go over the stack frame and see if it's been called but there's a much simpler method. You have a stack frame for each thread. So make the static variable [ThreadStatic]

[ThreadStatic] static bool _disable = false;

How does this work?

http://msdn.microsoft.com/en-us/library/system.threadstaticattribute.aspx

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

Edit: that alone might not be enough, however. What you probably want is one static variable per TASK. Now, as tasks would be executed sequentially per thread, in this particular case I don't think it's an issue, unless loggers can potentially fail without disabling... and I'm not sure what happens in that case, but it might require you to at the very least wrap things in a try/finally block:

static void Main() //Main(string[] args)
{
    string[] messages = "this is a test.  but it's going to be an issue!".Split(' ');

    Parallel.ForEach(messages, Log);

    Console.ReadLine();
}
[ThreadStatic]
static bool _disable = false;
public static void Log(string message)
{
    if (_disable)
    {
        return;
    }
    try {
        _disable = true;
        Console.WriteLine(GetPrefix() + message);
    } finally {
        _disable = false;
    }
}

public static string GetPrefix()
{
    Log("Getting prefix!");
    return "Prefix: ";
}

Edit II: From http://msdn.microsoft.com/en-us/library/dd460712.aspx it seems that once any of a set of tasks throws an exception outside of the task delegate, you are not guaranteed execution of any remaining tasks. It's best to handle those exceptional cases in your delegate.

+1: This works well with the contrived example. Looks like there's also [an equivalen for instance fields in .NET 4.0](http://theburningmonk.com/2010/10/threadstatic-vs-threadlocal/). I will play with this and see how it behaves and come back with feedback. Thanks! — zimdanen, Jul 18 '13 at 21:02

score 0 · Answer 2 · answered Jul 18 '13 at 17:23

0

How about splitting the log method into:

public static void LogWithPrefix(string message)
{
    var prefix = GetPrefix();
    Log(prefix + message);
}

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

answered Jul 18 '13 at 17:23

Reda

2,289
17
19

The prefix is required. In this case, getting the prefix represents getting the logging settings. Without those, I don't know what or where to log. – zimdanen Jul 18 '13 at 17:25
@zimdanen: So, then, where is `Log("Getting prefix!");` supposed to log to in your original example? – JayC Jul 18 '13 at 18:15
@JayC: I'd like to disable it so it doesn't log anywhere. – zimdanen Jul 18 '13 at 18:25

Circular dependency with cross-cutting concern

2 Answers2

Linked