3

What I'm trying to do

public void SomeTestMethod(){
    var person = this.PersonManager.Get(new Guid("someguid")); 

    person.Lastname = "Jameson";

    this.PersonManager.Save(person); // This goes wrong
}

Where it goes wrong

The above save method calls this code:

protected void Add<T>(T source, MyEntities context, bool isNew) where T : class
{
    if (isNew)
    {
        context.Set<T>().Add(source);
    }
    else
    {
        var entry = context.Entry(source);
        if (entry.State == EntityState.Detached)
        {
            context.Set<T>().Attach(source);

            entry.State = EntityState.Modified;
        }
    }
}

The var entry = context.Entry(source); line is the one causing this error:

The operation cannot be completed because the DbContext has been disposed.

I've seen answers to similar questions that suggested to use .ToList() (or something else to execute the link), but that has happened, because the Get returns a DTO object.

Some background

The PersonManager used in save, uses this to set the DbContext:

var context = new MyEntities();
this.PersonRepository = repositoryProvider.GetRepositoryByType<PersonRepository>(context);

The var context = new MyEntities(); is just to get it working now, this will be DI injected.

Which in turn depends on this:

public T GetRepositoryByType<T>(MyEntities context) where T : IContextDependent
{
    var instance = this.Repositories.SingleOrDefault(x => x is T);

    instance.SetContext(context);

    return (T)instance;
}

As the same PersonManager is used, de facto the same PersonRepository is used (and as a result the same context), so I don't see why it would be disposed on the second call.

Spikee
  • 3,967
  • 7
  • 35
  • 68

1 Answers1

3

You haven't given the context of where you're creating your context, but I'm assuming it's in a method, perhaps the constructor. Your context is scoped to that method, so the GC is free to dispose of it when the method call ends. I think the fact that it works at all, even just once, is that you're managing to hit it before it's garbage collected.

Ironically, your issue is occurring because you're not using DI yet. Simply injecting it would be enough likely to solve the issue. At the very least, your context should be scoped at the same level as PersonManager.

Chris Pratt
  • 232,153
  • 36
  • 385
  • 444
  • Correct, I'm using constructor injection. Please see my update regarding DI. – Spikee Dec 15 '15 at 13:55
  • The GC never ever directly calls `.Dispose()`. The GC may call a finalizer and that may explicitly call `.Dispose()`, but in most cases the GC doesn't trigger a dispose. – Enigmativity Dec 15 '15 at 13:59
  • @Enigmativity: I wasn't speaking literally. I only meant that the context is eventually disposed by the work of the GC, not necessarily that GC actually called `Dispose` on it. – Chris Pratt Dec 15 '15 at 14:09
  • 1
    @Spikee: That's a totally seperate issue, that would technically warrant its own question, but suffice to say, it boils down to how you're handing the add. There's some issue there, combined likely with something else going on in other code (implicitly adding an entity by adding it to a collection, etc.) – Chris Pratt Dec 15 '15 at 14:14
  • 1
    Just tested this and I can confirm. If I return a context from a method like Spikee does, I can fetch some entities. If I let my thread sleep 5 seconds to wait a bit, I can't fetch entities anymore with the context which indicates it's been disposed. – Alexander Derck Dec 15 '15 at 14:15
  • Accepted as answer. Completing the DI got me further. There is still an issue, but that's off topic for this question (and likely very specific). See here: http://stackoverflow.com/questions/34291602/unable-to-save-because-of-another-entity-of-the-same-type-already-has-the-same – Spikee Dec 15 '15 at 14:28
  • @ChrisPratt - I wasn't clear enough. If you explicitly know that a finalizer exists and that it calls `.Dispose()` then the GC will eventually dispose of your object. Otherwise the GC never ever calls `.Dispose()`. So when you say, "context is eventually disposed by the work of the GC", that would be wrong. In the case of EF I don't believe that the finalizer calls `.Dispose()` so the GC never will. – Enigmativity Dec 15 '15 at 22:22
  • @ChrisPratt - I just checked on the actual implementation of `DbContext` and there's no finalizer. So the GC definitely never calls `.Dispose()`. – Enigmativity Dec 15 '15 at 22:43
  • @Enigmativity: Okay, again, that's not what I meant. "Disposed", "destroyed", "discarded", "round-filed", "flushed-down-the-toilet", whatever you want to call it. My point was that it doesn't exist any more due to the GC. How it gets that way, has no bearing whatsoever on this discussion, so it's just semantics. – Chris Pratt Dec 16 '15 at 14:16
  • @ChrisPratt - Sorry to be painful, but the question is specifically about the context being disposed - and that is through a call to `.Dispose()`. So to say that "the GC is free to dispose of it" is, in this particular case, quite a misleading word to use. Hence my comments. – Enigmativity Dec 16 '15 at 23:22
  • @Enigmativity: It's actually the wording of the exception that is misleading. You get the same exception whether `Dispose` was literally called or not. Even if it was just garbage collected, the exception still says it was disposed. My generality of my language was following the generality of the exception. And now, we've devoted an obscene amount of text to something that doesn't even matter. – Chris Pratt Dec 17 '15 at 14:57
  • @ChrisPratt - Again, sorry to be painful, but if it is GC'ed how do you keep a reference to be able to get the exception? – Enigmativity Dec 17 '15 at 22:33
  • Because the GC just recovers the memory. If you were working in something like C, we're talking about the equivalent of a null-pointer exception. The application knows there's a variable that once contained a reference to a context instance, but now the data is gone. The exception assumes it was disposed, since that's the most likely scenario. Remember we're not supposed to be creating deliberate memory leaks. – Chris Pratt Dec 18 '15 at 15:10