CODEWITHSUNDEEP
c#
Miguel Moura
Miguel Moura

Reputation: 39364

Implement Singleton ... With Lock?

On a multithread application (ASP.NET MVC) I need to have a global settings class which contains constants and values taken from Web.Config.

I would like to have this class static, as singleton ... And locked?

public static class Settings {

  public static LoggerSettings Logger;
  public static MailerSettings Mailer;

  public class LoggerSettings {
    public String Levels { get { return ConfigurationManager.AppSettings["Logger.Levels"]; } }
    public const String Report = "[email protected]";
  } // LoggerSettings

  public class MailerSettings {
    public String Contact { get { return ConfigurationManager.AppSettings["Mailer.Contact"]; } }
  } // MailerSettings

}

I think I should implement a double lock? No?

I am not sure the best way to do this. Could I, please, get some help?

Thank You, Miguel

Upvotes: 1

Views: 1710

Answers (5)

pero
pero

Reputation: 4259

Take a look at Jon Skeet's article Implementing the Singleton Pattern in C#

Simplest and good enough option: 

public sealed class Settings
{
    private static readonly Settings instance = new Settings();

    public LoggerSettings Logger { get; private set; }
    public MailerSettings Mailer { get; private set; }

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static Settings()
    {
    }

    private Settings()
    {
       Logger = new LoggerSettings();
       Mailer = new MailerSettings();
    }

    public static Settings Instance { get { return instance;} }
}

public class LoggerSettings {

    public LoggerSettings()
    {
        Levels = ConfigurationManager.AppSettings["Logger.Levels"];
    }

    public String Levels { get; private set; }
    public const String Report = "[email protected]";

}

// Mailer settings would look similar

As you are only reading data from this instance you don't need any locking. The singleton instance is created before any other thread can access it so no need to lock there also.

Usage: 

 Settings.Instance.Mailer.Contact

Upvotes: 1

Eric Lippert
Eric Lippert

Reputation: 659994

I would like to have this class static, as singleton

To implement a singleton correctly in C#, see Jon Skeet's excellent summary of what does and does not work:

http://csharpindepth.com/Articles/General/Singleton.aspx

I think I should implement a double lock? No?

No. Double-checked locking is a low-lock technique and therefore insanely dangerous on weak memory model hardware. The moment you stray even the slightest from a "blessed" pattern you have abandoned all hope of the program behaving predictably.

The only circumstances under which I would use double-checked locking are when all of the following are true:

  • Is there is extensive empirical evidence that single-checked locking produces poor performance?
  • Let's suppose single-checked performance is unacceptable. Single-checked locking usually produces bad performance due to contention, so step one is eliminate the contention. Can you eliminate the contention and get acceptable performance? I would only use double-checked locking if it was impossible to remove the contention or if the performance problem was caused by the several nanoseconds it takes to obtain an uncontended lock. In the latter case: wow, that's a fast program that those nanoseconds are the slowest thing, and wow, you have pretty serious performance requirements if you're counting individual nanoseconds.
  • Let's suppose that single-checked performance is unacceptable and cannot be fixed. Is there another low-lock technique, like using Interlocked.CompareExchange or Lazy<T> that has acceptable performance? At least you know that CompareExchange and Lazy<T> were written by experts and enforce memory barriers appropriately on all hardware. Don't use double-checked locking if there is a better tool already implemented.
  • Let's suppose that none of these tools give acceptable performance. Does double-checked locking give acceptable performance? If not then don't use it.

Upvotes: 10

Peter Punzenberger
Peter Punzenberger

Reputation: 561

if you like it static and as Singleton, try it this way:

public static class Settings {
  private static readonly object LockObject = new object();
  private static LoggerSetting LoggerInstance;

  public static LoggerSetting LoggerSettings {
    get {
      lock (LockObject) {
         if (LoggerInstance == null)
           LoggerInstance = new LoggerInstance(); 

         return LoggerInstance;
       }
     }
   }

   public class LoggerSetting {
     public String Levels {
       get { return ConfigurationManager.AppSettings["Logger.Levels"]; }
     } 

     public const String Report = "[email protected]";
   }
}

and use it by:

string x = Settings.LoggerSEttings.Report;

Upvotes: 1

Vignesh.N
Vignesh.N

Reputation: 2666

if you'd still like to go ahead and have a singleton instance

    public class MySettings{
    private static Object lockObj = new Object();
    private MySettings() {} // private .ctor

    private static MySettings _instance;

    public static MySettings MySingleSettings{
     get{
      if(_instance == null){
      lock(lockObj){
        if(_instance == null)
          _instance = new MySettings();
        }
      }
       return _instance;
    }
}

Upvotes: -1

evgenyl
evgenyl

Reputation: 8107

as I see, you only read the data. So you need not lock here IMO. Use static constructor to init your variables, like Report (made it also static).

Upvotes: 5

Related Questions