9
\$\begingroup\$

I found out the hardway that access to DbContext in .NET is not threadsafe. I have a singleton for logging things using a dbcontext. The original version uses something like

public Logger{
  private MyContext context;
  private static Logger instance = new Logger();

  private Logger(){
    //init stuff, including context
  }

  public static Logger Instance {
      return this.instance;
  }

  public void someMethod(){
    //do something with this.context
  }
}

I'm thinking of a few solutions:

one would be is not making this a singleton at all, but instantiate a logger each time I need one.

I can think of added overhead as a disadvantage for this way, and simplicity as an advantage.

another one is locking (on the context or type) for each public method:

public void someMethod(){
  lock(this.context){
    //do something with this.context
  }
}

This adds extra maintenance complexity.

a third one could be one context per thread in a form of

private ConditionalWeakTable<Thread, MyContext> contexts = new ConditionalWeakTable<Thread, MyContext>();

private MyContext Context{
  get {
    return contexts.GetValue(Thread.CurrentThread, createContext());
  }
}

private MyContext createContext(){
  //instantiate a context
}
  • Pro: fairly consice, complexity is isolated
  • Con: batshit insane? Using System.Runtime.CompilerServices for something fairly mondaine, which also isn't what it's meant for.

Am I overlooking any arguments? What would you do?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ +1 for the con, although if you are looking at logging as 'mondaine' then why do you need a dbcontext with it? If you are actually logging to a database then in all honesty use a pre-made solution, will save you lots of time and effort. \$\endgroup\$
    – hyp
    Commented Sep 18, 2012 at 15:05

4 Answers 4

12
\$\begingroup\$

General rules of thumb:

  1. In order to take advantage of connection pooling (and you should), database connections should be as short lived as possible. Create, use, then immediately destroy.

  2. Single instance objects should always be agile (defined as not holding system resources e.g. db connections, file handles, etc.).

  3. If you need a single instance object to handle a system resource, it should be done in a protected manner using a named mutex (naming makes a mutex available to all threads across the computer) or Monitor. Within the protected block of code, if the resource is a db connection, apply rule 1.

  4. When using object locking, you should create your locks around an arbitrary object, not the current object's instance e.g.:

    private static readonly object _lockObject = new object();
    

    ...

    try {
        Monitor.Enter(_lockObject);
        // Do protected stuff...
    }
    finally {
        Monitor.Exit(_lockObject);
    }
    
\$\endgroup\$
1
  • 3
    \$\begingroup\$ I disagree with point 4. nothing wrong with locking the object you are using if it is private. \$\endgroup\$
    – markmnl
    Commented Dec 3, 2013 at 4:56
3
\$\begingroup\$

I believe that the dbcontext shouldn't be shared between multiple threads - I remember reading about that somewhere, but I can't find the link. Sharing a context between threads can lead to all sorts of problems, though in your case (a logger) that should be limited, cause as I understand it would be used as a 'push only' class.

My suggestion would be to either create the dbcontext each time you use it and wrapp it in a using statement or provide it as a dependency to the logger class and perhaps use an IoC container to control the lifetime of the context. Instantiating a context is not expensive - at least that's what the docs say.

Here are some links that may help:

how to decide on a lifetime for your objectcontext

Why re-initiate the DbContext when using the Entity Framework?

\$\endgroup\$
3
  • \$\begingroup\$ well, all these ideas were to avoid sharing the context between the threads; that is shouldn't be shared is just the problem I'm trying to fix. \$\endgroup\$
    – Martijn
    Commented Sep 18, 2012 at 20:12
  • \$\begingroup\$ the reason I have it static is to avoid problems with attaching objects from one context to another, which leads to headaches on add vs attach. Having multiple instances of the logging class would fix that though, that is solution #1 I propose above. \$\endgroup\$
    – Martijn
    Commented Sep 18, 2012 at 20:16
  • 1
    \$\begingroup\$ I can see the problems that you mention - that's why I'd suggest the IoC. If it's a web app then you can set the lifetime of the context to be per request which is (usually) handled by one thread - unless you're doing some async operations. If it's a win app then depending on the container you choose, you may want to experiment - maybe you'll find a container with a instance per thread lifetime and that should solve the problem. Of course having the context as an external dependency for the logger adds verbosity, but I'd choose that over struggling with the problems you mentioned any time. \$\endgroup\$
    – Chris
    Commented Sep 19, 2012 at 9:50
2
\$\begingroup\$

I'm thinking of a few solutions:

one would be is not making this a singleton at all, but instantiate a logger each time I need one.

I can think of added overhead as a disadvantage for this way, and simplicity as an advantage.

This is probably the right move. The overhead of instantiating a DbContext object is pretty minimal.

What would be ideal is if you created a DbContextFactory class to build you instances of your DbContext. Add a private readonly field for the factory and build a new context object for each database interaction.

Something like this

public class DbContextFactory
{
    private readonly string _connectionString;

    private static DbContextFactory _instance;
    public static DbContextFactory Instance
    {
        get 
        {
            if (_instance == null)
            {
                _instance = new DbContextFactory("MyConnectionStringOrWhatever");
            }

            return _instance;
        }
    }

    private DbContextFactory(string connectionString)
    {
        _connectionString = connectionString;
    }

    public DbContext BuildDbContext()
    {
        //or whatever you need to do to build your dbcontext
        return new DbContext(Instance._connectionString);
    }
}

And the

public class Logger
{
    private readonly DbContextFactory _dbContextFactory;

    private static Logger _instance;
    public static Logger Instance
    {
        get 
        {
            if (_instance == null)
            {
                _instance = new Logger();
            }

            return _instance;
        }
    }

    private Logger()
    {
        this._dbContextFactory = DbContextFactory.Instance;
    }

    public void SomeMethod()
    {
        using (var context = _dbContextFactory.BuildDbContext())
        {
            //do something with context
        }
    }
}
\$\endgroup\$
0
\$\begingroup\$

DBContext also has a cache that I would like to leverage for queries behind a WebAPI. Would any of these allow the threads of the WebAPI to use the same DBContext and provide the response via the DBContext cache? I would likely use a timed refresh to ensure the cache is refreshed, as most of the queries are for reference data.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.