Ayende @ Rahien

My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:


+972 52-548-6969

, @ Q c

Posts: 6,026 | Comments: 44,842

filter by tags archive

Find the bugAccidental code reviews

time to read 1 min | 183 words

I was working with a client about a problem they had in integrating EF Prof to their application, when my caught the following code base (anonymized, obviously):

public static class ContextHelper
     private static Acme.Entities.EntitiesObjectContext _context;

     public static Acme.Entities.EntitiesObjectContext CurrentContext
           get { return _context ?? (_context = new Acme.Entities.EntitiesObjectContext()); }


That caused me to stop everything and focus the client’s attentions on the problem that this code can cause.

What were those problems?

More posts in "Find the bug" series:

  1. (11 Sep 2015) The concurrent memory buster
  2. (20 Apr 2011) Why do I get a Null Reference Exception?
  3. (25 Nov 2010) A broken tree
  4. (13 Aug 2010) RavenDB HiLo implementation
  5. (25 Jul 2010) Accidental code reviews



Thread safety, first of all.

Second, there are many points against singletons which I'm too lazy to iterate, especially since there are cases where singletons are exactly the right thing.

What's wrong with

public static class ContextHelper {

public static readonly EntitiesObjectContext CurrentContext = new EntitiesObjectContext();

} ?


Could it be a race condition where more than one context gets created?


Does the 2nd half of the ?? Operator even return the correct type?


There's a missing closing curly brace, though I assume that's a typo

Thilak Nathen

Object contexts are reasonably volatile resources... like NH sessions. Not a good idea to singleton them. They are meant for short lived usages.

Wile E. Coyote

this is why I never catch that @#$@ bird


It’s not even a singleton. This code shows that EntitiesObjectContext may have internal/ public constructor.


Nice :)

a) My # Re to 'configurator':

With code like

public static class ContextHelper {

   public static readonly EntitiesObjectContext CurrentContext = new  EntitiesObjectContext();


nothing wrong, except that CurrentContext created when app starts instead of "lazy" way from original code with C# "getter"... While sure it's sometimes maybe make sense to create objects right after App start, usually better to put such logic into bootstrappers.. and I never see that contexts (entities contexts) somebody create such a way, i.e. ones after app starts :D and keep forever... but sure as always "it depends" :)

b) with original code from famous post author issue that I see, like in most of real world situations, is that programmer probably want CurrentContext to be actually "current", not the one that was created when you first time use CurrentContext in blog post code... :)

c) Sure no thread safety at all.. 100% agree! but I was think that it's normal to see "warn" sign when you see static fields and no "lock" in the moment when you fill them :)


Thread unsafe work with shared variable . In such situation it's possible that concurrent clients get diffrent instances of the context class.


Yeah, probably EntitiesObjectContext:System.Data.Objects.ObjectContext and they decided to reuse its instance, but there was some threading problem, ‘cause the app was an ASP.NET app or something.


nothing wrong, except that CurrentContext created when app starts

No, the initialization is lazy : the static constructor is called when a reference to the class is found by the JIT.


The ObjectContext is a Unit of Work, and it's completely bonkers to make it static. Any thread safety or race condition issues are irrelevant in comparison

Ayende Rahien


Thread Safety is a problem, sure.

But let us assume that we are talking about single threaded code, it is still wrong, why?

Ayende Rahien


You are trying to find a bug in the dark, when it is sitting under the lamp and chuckling.

In other words, don't try to find non obvious / complex ones, this one has glaring issues.

Ayende Rahien

Thilak & gunteman,


Steve Py

As mentioned above, If their context implementation is an object context then it should not be treated as a Singleton instance.

Still I'm pretty sure that modified code doesn't work as they intended (aside from the missing brace) It's probably supposed to be something like: [Namespaces stripped out for clarity]

public static EntitiesObjectContext CurrentContext




    _context = _context ?? new EntitiesObjectContext();

    return _context;



Additional little unpleasant smells: (Not context specific) Returning concrete types. Locks implementation details in. I'd define an interface for the context and have a factory create a specific concrete instance. Better yet I'd configure an IOC container to resolve it as a dependency of whatever class needed to avoid hand-writing factories.

Ayende Rahien


return context ?? (context = new Acme.Entities.EntitiesObjectContext())

Is equal to:

if(_context == null)

_context = new Acme.Entities.EntitiesObjectContext();

return _context;

Joe Campbell

Which implies a double checked locking issue?

Steve Py

So it is.... That just looks so wrong. :)

Steve Py

I certainly hope this wasn't an ASP.Net application.

Mike Minutillo

With EF4 the context performs change tracking by keeping a reference to each object it produces so over time you'll end up with every entity in your database in memory.

Potentially unrelated parts of the app can mess with each other by calling SaveChanges() at inappropriate (unexpected) times persisting changes which might be incomplete. Transactions would be an absolute nightmare.

Kelly Stuard

Side note: It's odd that I don't see anything related to the newly-released NHibernate 3.0.0Alpha1.


With a static context we will get a memory leak since the context is acting as a cache for entities that were returned from the database. Also, it creates a state entry for each entity and additional objects to track entity relationships which of course in the long run will fill the memeory and crush the application.


Ok, All context objects will stay in the cache after the first access, after some time we'll have all database objects in cache(objectcontexts objects). Am I right? I just want to know the right way to do this with objectcontext. On the L2Sql i putted the DataContext in HttpContext.item, it was hardly stressed, more then 5000 updates, inserts per second, 10 users sim and never lost a transaction. Now i wanna port linq 2 sql to EF. Wich one will be the right way or better way?

Gian Maria Ricci

It will use the same EntityObjectContext for the whole application, this can cause problem if multiple thread will access the context. But the error is because this is like using the same session in all the program, what happens if during a flush we have an exception? After an exception in a flush / update we need to change the context, because it is no more usable.



Potential Problems:

  1. Bad implementation of a singleton, if indeed that was the intention.

  2. Has potential to cause problems (data integrity and possible race condition) if the same session uses multiple threads.

  3. Static implementation will stay in scope and not garbage collect until the thread shuts down.

  4. Readonly property will not allow a graceful recovery if an exception happens.

Alois Kraus

I did not much tinker with databases but I would ask the question why the CurrentContext has no set property?

If I ever choose to open another DB I would like to use a new context. Since it is a static class I think that there is currently no way to inject a fresh context to the underlying object chain.

Just my non educated guess.


Alois Kraus

Comment preview

Comments have been closed on this topic.


No future posts left, oh my!


  1. Technical observations from my wife (3):
    13 Nov 2015 - Production issues
  2. Production postmortem (13):
    13 Nov 2015 - The case of the “it is slow on that machine (only)”
  3. Speaking (5):
    09 Nov 2015 - Community talk in Kiev, Ukraine–What does it take to be a good developer
  4. Find the bug (5):
    11 Sep 2015 - The concurrent memory buster
  5. Buffer allocation strategies (3):
    09 Sep 2015 - Bad usage patterns
View all series


Main feed Feed Stats
Comments feed   Comments Feed Stats