A static property in static class when used concurrently

Question

I have a static class 'Logger' with a public property called 'LogLevels' as in code below.

When the property is used concurrently in a multi-user or multi-threaded environment, could it cause problems?

Do I need to use thread synchronization for the code within the property 'LogLevels'?

 public class Logger
{
    private static List<LogLevel> _logLevels = null;


    public static List<LogLevel> LogLevels
    {
        get
        {
            if (_logLevels == null)
            {
                _logLevels = new List<LogLevel>();
                if (!string.IsNullOrWhiteSpace(System.Configuration.ConfigurationManager.AppSettings["LogLevels"]))
                {

                    string[] lls = System.Configuration.ConfigurationManager.AppSettings["LogLevels"].Split(",".ToCharArray());
                    foreach (string ll in lls)
                    {

                        _logLevels.Add((LogLevel)System.Enum.Parse(typeof(LogLevel), ll));
                    }
                }
            }

            if (_logLevels.Count == 0)
            {
                _logLevels.Add(LogLevel.Error);
            }
            return _logLevels;
        }
    }
}

UPDATE: I ended up using thread synchronization to solve concurrency problem in a static class, as in code below.

public class Logger
{
    private static readonly System.Object _object = new System.Object();

    private static List<LogLevel> _logLevels = null;


private static  List<LogLevel> LogLevels
    {
        get
        {
            //Make sure that in a multi-threaded or multi-user scenario, we do not run into concurrency issues with this code.
            lock (_object)
            {
                if (_logLevels == null)
                {
                    _logLevels = new List<LogLevel>();
                    if (!string.IsNullOrWhiteSpace(System.Configuration.ConfigurationManager.AppSettings["SimpleDBLogLevelsLogger"]))
                    {

                        string[] lls = System.Configuration.ConfigurationManager.AppSettings["SimpleDBLogLevelsLogger"].Split(",".ToCharArray());
                        foreach (string ll in lls)
                        {

                            _logLevels.Add((LogLevel)System.Enum.Parse(typeof(LogLevel), ll));
                        }
                    }
                }

                if (_logLevels.Count == 0)
                {
                    _logLevels.Add(LogLevel.Error);
                }
            }
            return _logLevels;
        }
    }
}

Your property is `private` in your example, not `public` – JerKimball Mar 01 '13 at 18:38 — JerKimball, Mar 01 '13 at 18:38
I have corrected the modifier and changed it to public. – Sunil Mar 01 '13 at 21:38 — Sunil, Mar 01 '13 at 21:38

score 3 · Accepted Answer · answered Mar 01 '13 at 18:37

3

When the property is used concurrently in a multi-user or multi-threaded environment, could it cause problems?

Absolutely. List<T> is not designed for multiple threads, except for the case where there are just multiple readers (no writers).

Do I need to use thread synchronization for the code within the property 'LogLevels'?

Well that's one approach. Or just initialize it on type initialization, and then return a read-only wrapper around it. (You really don't want multiple threads modifying it.)

Note that in general, doing significant amounts of work in a static constructor is a bad idea. Are you happy enough that if this fails, every access to this property will fail, forever?

answered Mar 01 '13 at 18:37

Jon Skeet

1,421,763
867
9,128
9,194

...unless nobody can access it (I think he meant it to be public, not private) – JerKimball Mar 01 '13 at 18:38
Jon - So, you are saying to either use thread synchronization Or transfer the property code to a static constructor for this class. Right? Or you meant something else? – Sunil Mar 01 '13 at 18:45
@Sunil: Yes, but most importantly return a read-only wrapper. – Jon Skeet Mar 01 '13 at 18:47

score 3 · Answer 2 · answered Mar 01 '13 at 18:38

This code posses race conditions and cannot be safely executed from multiple threads. The primary problem is the List<T> type is not thread safe yet this code will freely write to. This mean the writes can occur in parallel and hence break the implicit contract of List<T>

score 1 · Answer 3 · answered Mar 01 '13 at 18:39

1

The short answer is "yes" and "yes" you do need threads synchronization. The other question is, why re-invent the wheel? You can use something like log4net or .NET logging framework.

answered Mar 01 '13 at 18:39

user1873415

41
3

The problem with all these logging frameworks is the steep learning curve. I want my developers to start logging from day 1, and all I want is logging to a database. – Sunil Mar 01 '13 at 20:11
You can take it slowly. The basic feature of log4net, for instance, as well as its basic configuration, is relatively simple and even novice can start using it understand it within couple of hours. Or you can create a simplified wrapper around lon4net, that will hide the configuration complexity, – user1873415 Mar 01 '13 at 20:25

A static property in static class when used concurrently

3 Answers3