0

I am working on an existing C# web app using ASP.NET MVC, targeting .NET 4.5. It uses Unity for IoC, with a custom lifetime manager that appears to be equivalent to per request (I believe it was set up before PerRequestLifetimeManager was added to Unity). The project also uses Entity Framework to talk to a SQL Server database, and a WCF client to talk to a webservice.

The types that are registered are: several custom service interfaces mapped to their classes (none of these are currently implementing IDisposable), an interface mapped to a derived class of the WCF client, and a derived class of the DBContext.

I'm fairly new to IoC; I've done some reading in preparation for adding a logging task and I think there are existing problems in the code base. I'll post a follow-up question about my planned changes later, but I want to make sure the existing code is correct first.

The service classes get their WCF- and DBContext-derived classes injected as properties:

[Dependency]
public IMyWCFClient MyWCFClient { get; set; }
[Dependency]
public MyDBContextDerivedClass MyDBContext { get; set; }

I believe this is okay for the DBContext property since it doesn't actually need to be Dispose'd, but for the WCF client I think it will never get properly cleaned up - Close() is never called manually, Dispose() is never called manually, and the service doesn't implement IDisposable. Further, it seems from my reading that implementing IDisposable on the service and having it call Dispose() on its WCF client property would be wrong, and instead we need to call Close(), check many exceptions, and call Abort() if necessary.

Am I correct in thinking the current implementation is a problem?

If so, I've found the ChannelAdam WCF library (https://www.nuget.org/packages/ChannelAdam.Wcf) and it looks like a promising and hopefully-not-too-painful way of fixing this issue - is this a reasonable approach?

edit: Also, am I right that I don't need to worry about cleaning up the DBContext?

Chelsar
  • 373
  • 2
  • 8
  • No, you are not right that you don't need to worry. You should be worried, each time your service is activated, you create a new instance of the DbContext that is not disposed. This could possibly lead to leaks of unmanaged resources. – Wiktor Zychla Jan 13 '17 at 08:12
  • According to this thread: http://blog.jongallant.com/2012/10/do-i-have-to-call-dispose-on-dbcontext/ The author spoke with the EF developer team and found that it's not actually necessary to call `Dispose()` on a `DBContext`. I think it's still a good idea just for the sake of following best practices, since it does implement `IDisposable` (and in case we ever end up with derived classes where it would cause a problem), but my reading suggests we're not going to end up with leaks if we don't. – Chelsar Jan 13 '17 at 16:19
  • 1
    The 4 years old thread could be a source of information but their implementation from https://github.com/aspnet/EntityFramework6/blob/master/src/EntityFramework/DbContext.cs sounds like a much better one. After just a sneak peek I would be worried, some edge cases seem to require Dispose. – Wiktor Zychla Jan 13 '17 at 16:35
  • 1
    Fair enough. I'll be making the services implement `IDisposable` and I'll make sure they clean up the `DBContext`, along with whatever we end up doing to safely manage the WCF clients. – Chelsar Jan 13 '17 at 16:42

0 Answers0