Does this implementation of the Entity Framework leaks memory?

Question

I just can't make out if the entity context is disposed in the usage flow when used in a using statement in a web application or a console application.

Thanks!

using System;
using System.Web;

namespace Foo.Model
{
    public partial class FooEntities : ObjectContext
    {
        private const string CurrentContextKey = "FooEntities.Current";

        [ThreadStatic]
        private static FooEntities _currentOnThreadStatic;
        private FooEntities _previousContext;

        /// <summary>
        /// Gets the current <see cref="FooEntities"/> instance, if an instance can be shared in the current context.
        /// </summary>
        /// <remarks>
        /// The current context is stored in the HTTP context, if it is available (otherwise it is stored in a thread-static instance).
        /// Multiple contexts can be stacked.
        /// </remarks>
        public static FooEntities Current
        {
            get
            {
                if (HttpContext.Current != null)
                {
                    return HttpContext.Current.Items[CurrentContextKey] as FooEntities;
                }
                else
                {
                    return _currentOnThreadStatic;
                }
            }

            private set
            {
                if (HttpContext.Current != null)
                {
                    HttpContext.Current.Items[CurrentContextKey] = value;
                }
                else
                {
                    _currentOnThreadStatic = value;
                }
            }
        }

        /// <summary>
        /// Returns a repository instance bound to this object context.
        /// </summary>
        /// <typeparam name="TRepository">The type of repository to instantiate.</typeparam>
        /// <returns>The repository instance.</returns>
        public TRepository GetRepository<TRepository>()
            where TRepository: BaseRepository
        {
            return (TRepository) Activator.CreateInstance(typeof(TRepository), this);
        }

        /// <summary>
        /// Ensures that an ambient context is available through <see cref="Current"/>, throwing an exception otherwise.
        /// </summary>
        /// <exception type="InvalidOperationException)">
        /// Thrown if <see cref="Current"/> is null.
        /// </exception>
        public static void EnsureContext()
        {
            if (Current == null)
            {
                throw new InvalidOperationException("An ambient FooEntities context is expected.");
            }
        }

        /// <summary>
        /// Releases the context instance.
        /// </summary>
        /// <param name="disposing"></param>
        protected override void Dispose(bool disposing)
        {
            Current = _previousContext;
            base.Dispose(disposing);
        }

        /// <summary>
        /// Is called by all constructors.
        /// </summary>
        partial void OnContextCreated()
        {
            _previousContext = Current;
            Current = this;
        }
    }
}

Do you have a reason for trying to store an ObjectContext inside your HttpContext? A context is considered light-weight, typically you create an instance when you need it, then dispose of it. — Joel C, Jul 28 '11 at 19:40
So FooEntities.Current may be used to in diffident classes so you don't have to inject an instance in the constructor and keep the same instance across all those classes. — Four, Jul 28 '11 at 20:08
I think you should read this one: http://stackoverflow.com/questions/3653009/entity-framework-and-connection-pooling/3653392#3653392 That would hopefully push you to reconsider that thread static context. — Ladislav Mrnka, Jul 28 '11 at 21:15
@Ladislav Mrnka: I think the implementation id c*&p. I don't support it, nor did I write it, nor imagine it. I believe that the entity context should be injected into the classes that need to access the model and the top level request handler should instantiate it. It is ugly ugly ugly! Bottom line I agree with you and hope no one takes it as an example of what to do but rather what _not_ to do. — Four, Jul 29 '11 at 15:29

score 2 · Accepted Answer · answered Jul 28 '11 at 20:06

2

It is an odd design. As @Joel C points out in his comment you should regard the object context as a shortlived object that you create when you need it and release right afterwards.

But I see no reason that this would leak memory. You are only dealing with managed resources and you are using the same key all the time to the HttpContext so you won't create new objects all over.

answered Jul 28 '11 at 20:06

Anders Abel

64,109
15
143
213

ObjectContext is IDisposable, so it's not a managed resource. That's why I worry about memory leaks. – Four Jul 28 '11 at 20:12
Have a look at the OnContextCreated method which is called in the constructor of the other partial of the class. – Four Jul 28 '11 at 20:14
@Four: Even if it is `IDisposable` it will be correctly freed by the garbage collector. `IDisposable` is an option (that should be used) to release expensive resources early, instead of waiting for the GC, but you won't introduce a leak if you ignore it. – Anders Abel Jul 28 '11 at 20:15
1

@Four: Sorry, didn't see the `OnContextCreated` before, but I still think it's fine because the "anchor" keeping the linked list alive is `HttpContext` which itself is very short lived. – Anders Abel Jul 28 '11 at 20:18
@Anders: The primary use of this interface is to release unmanaged resources. ... Furthermore, the garbage collector has no knowledge of unmanaged resources such as window handles, or open files and streams. http://msdn.microsoft.com/en-us/library/system.idisposable.aspx – Four Jul 28 '11 at 20:18
2

It's more like a memory "balloon" than a memory leak. Everything will eventually get freed, so it's not technically a leak. But the more entities that get loaded into the context before it's eventually disposed, the more memory will be taken up for change tracking, tracking relations, etc. – Joel C Jul 28 '11 at 20:19
In a correct implementation of `IDisposable`, the object finalizer ensures that any unmanaged resources are released. The finalizer is automatically called by the GC, so no, you can safely ignore `IDisposable`. However it is best practice to dispose of them instead of waiting for the GC. – Anders Abel Jul 28 '11 at 20:20
1

@Anders not really, the finalizer ensures *managed* resources are released. If you're using *unmanaged* resources such as handles, you're responsible for doing that yourself, and not disposing of your object properly will cause memory leaks. But it's most commonly used with managed resources to ensure that things are disposed of in a timely manner instead of waiting for GC, such as for database connections. – Joel C Jul 28 '11 at 20:24
@Anders and @ Joel: Since the HttpRequest is shot lived and it was my feeling too that it looked more like a memory balloon than a leak. It answers my question. Thanks guys. By the way It's not my code... (had to mention it) – Four Jul 28 '11 at 20:25
1

@Joel: If the ordinary pattern for implementing `IDiposable` is followed, the finalizer calls `Dispose(false)` which should release any unmanaged resources. The `false` flag tells `Dispose(bool disposing)` that it should ignore any managed references and only release unmanaged resources. – Anders Abel Jul 28 '11 at 20:28
This discussion made me [ask a question](http://stackoverflow.com/questions/6865330/will-ignoring-idisposable-cause-memory-leaks) where the details are sorted out. – Anders Abel Jul 28 '11 at 20:53

Does this implementation of the Entity Framework leaks memory?

1 Answers1

Linked