5

I know the caching of DbContext is not good idea. But I would like to do it fine. What do you think about this way?

public class Context : DbContext
{
    private Context()
    {
    }

    private static WeakReference<Context> _cachedContext;

    public Context Instance
    {
        get
        {
            Context context;
            if (!_cachedContext.TryGetTarget(out context))
            {
                context = new Context();
                _cachedContext.SetTarget(context);
            }
            return context;
        }
    }
}

This code is planned to be used without IDisposable.Dispose calling in the client-side. What problems this can cause except singleton (anti)pattern? Thanks.

Serg046
  • 1,043
  • 1
  • 13
  • 42
  • 1
    The general rule of thumb for a context is to create is as late as possible and kill it as quickly as possible. There is no reason to keep it hanging around. – DavidG Dec 04 '15 at 11:28
  • Why would cache the DbContext class? There is no reason why you should do this. There are other ways to manage the disposal of your DbContext class like DI containers (unity is just one to name). – hbulens Dec 04 '15 at 11:32
  • Your WeakReference singleton looks super-dodgy to me. If the only thing hanging on the context is a WeakReference, then it will be eligible for GC. You'll need a strong reference, not a weak one. Actually, because of this, your architecture will be accidentally better than the one you thought you were proposing, because occasionally you'll actually get a new context (but also probably some very confusing bugs). Better still, don't hold on to your context at all. It's a far more substantial anti-pattern than singleton. – spender Dec 04 '15 at 11:41
  • The easiest way to write a singleton is by leveraging `Lazy`. – spender Dec 04 '15 at 11:43
  • @hbulens Please see comment below. And I know about DI. It was just test sample. – Serg046 Dec 04 '15 at 11:45
  • @spender if I will use strong reference my db connection will not be disposed. It's not good. – Serg046 Dec 04 '15 at 11:46
  • Relying on the GC to do your disposing for you is also not good. – spender Dec 04 '15 at 11:47

2 Answers2

19

The DbContext is a cache. Keeping hold of it for a long time is a terrible idea... it will slowly consume your application's memory hanging on to data that may well be stale.

It was not designed to be used in the way you propose.

Don't do it.

A DbContext is a transient object that should be used and disposed of in the smallest scope possible.

 using(var ctx = new MyDbContext())
 {
      //make some changes
      ctx.SaveChanges();
 }

That's how it was designed to be used. Use it properly.

spender
  • 117,338
  • 33
  • 229
  • 351
  • 2
    @CodeCaster A strong consensus has been reached :) – spender Dec 04 '15 at 11:33
  • I'm not keeping an object. I'm keeping just weak reference. It does not prevent to work of Garbage Collector. But according to you tips I need avoid even the caching way above (by weak reference). Ok. I thought that when I create new instance of DbContext I have a new connection to db and It can affect performance – Serg046 Dec 04 '15 at 11:43
  • @Serg046 A new DbContext does not create a new connection to the database. Connections to the database are managed by other parts of the framework... the DbContext will borrow a connection from a pool. This leads to another problem if you don't dispose of your context quickly... the connection pool can become confused and exhausted. – spender Dec 04 '15 at 11:46
  • 1
    @Serg046 A DbContext is lightweight and inexpensive to construct. The cost of construction vs the cost of the query is tiny. Don't try to optimise away a non-existent problem by skiing off-piste. – spender Dec 04 '15 at 11:48
  • thank you, looks like that creating of dbcontext is very simple operation and it's wrapper of existing connection Do we have official information about caching strategy of db connection? – Serg046 Dec 04 '15 at 11:50
7

This is an XY problem. Why do you want to "cache" the DbContext? What benefit do you think you'll gain from this?

You should not do this, ever. The class was not meant for this. It will instead cause performance problems (undoing the benefit you think to gain) and persistent errors after you attached invalid entities - you'll never be able to save entities using this context again, as the change tracker holds the entities.

See Correct usage of EF's DBContext in ASP.NET MVC application with Castle Windsor, Working with DbContext, Managing DbContext the right way with Entity Framework 6: an in-depth guide, Manage the lifetime of dbContext or any of the other thousands of hits on searching the web for "entity framework dbcontext lifetime".

If you want to cache data, then instead cache the records themselves. There are existing solutions (code and libraries) that can help you with this, which is called "second level caching". No need to write it yourself. See for example How to make Entity Framework cache some objects.

Community
  • 1
  • 1
CodeCaster
  • 147,647
  • 23
  • 218
  • 272