CODEWITHSUNDEEP
c#.netnhibernatedependency-injectionclr
sentinel21
sentinel21

Reputation: 546

How to find out where my legacy application using NHibernate is leaking memory

I have a legacy application that I'm maintaining that is leaking memory.
I am reasonably confident that the source is the session management/dependency injection code. It uses Simple Injector and NHibernate.

To start, here are some helper classes and interfaces we use: 

public class SessionFactory : Dictionary<string, Func<ISession>>,Helpers.ISessionFactory, IDisposable
{
    public ISession CreateNew(string name)
    {
        return this[name]();
    }

    public void Dispose()
    {
        foreach (var key in Keys)
        {
            this[key]().Close();
            this[key]().SessionFactory.Close();
        }
    }
}

public interface ISessionFactory
{
    ISession CreateNew(string name);
}

Here is what the container initialization looks like:

    private static void InitializeContainer(Container container)
    {

        var connectionStrings = System.Configuration.
            ConfigurationManager.ConnectionStrings;
        var sf1 = new Configuration().Configure().SetProperty(
            "connection.connection_string",
            connectionStrings["db1"].ConnectionString
            ).BuildSessionFactory();
        var sf2 = new Configuration().Configure().SetProperty(
            "connection.connection_string",
            connectionStrings["db2"].ConnectionString
            ).BuildSessionFactory();
        var sf3 = new Configuration().Configure().SetProperty(
            "connection.connection_string",
            connectionStrings["db3"].ConnectionString
            ).BuildSessionFactory();


        container.Register<ISessionFactory>(() =>
            new SessionFactory
            {
                {"db1", sf1.OpenSession},
                {"db2", sf2.OpenSession},
                {"db3", sf3.OpenSession}
            }, Lifestyle.Scoped);
    }

Then, inside our base controller (other controllers inherit from it), this happens:

protected BaseController(ISessionFactory factory)
{
    this.factory = factory;
    db1Session = factory.CreateNew("db1");
    db2Session = factory.CreateNew("db2");
    db3Session = factory.CreateNew("db3");
}

From there, all of our methods can use a session from any database. Some request methods use multiple database sessions to complete their tasks. This project does not utilize the repository pattern at this point -- rewriting it would be an expensive operation. Is there any obvious memory leak I'm missing in this code?

Upvotes: 0

Views: 194

Answers (2)

Steven
Steven

Reputation: 172626

I find your design very suspicious. First of all, your factory is leaking connections, since although you try to dispose it, the only thing you achieve is disposing things you just opened during disposal; this isn't very useful and means the already created connections will not be closed. Second, a design where your application requests the proper connection using a string based approach is error prone. Your application is probably dealing with multiple database schemas, where each connection relates to a certain schema. This means that connections aren't interchangeable and this warrants the use of a unique abstraction per schema. So instead of having one generic ISessionFactory abstraction that tries to serve all consumers (and currently fails), make things explicit by giving each unique schema its own abstraction. For instance:

public interface IDb1SessionProvider
{
    ISession Session { get; }
}

public interface IDb2SessionProvider
{
    ISession Session { get; }
}

public interface IDb3SessionProvider
{
    ISession Session { get; }
}

By lack of context, I named the interfaces IDbXSessionProvider, but I bet you can come up with a better name.

This might look weird, since all interface have the same method signature, but remember that they have each a very different contract. The Liskov Substitution Principle describes that they should not share the same interface.

An implementation for such provider can be made as follows:

public class FuncDb1SessionProvider : IDb1SessionProvider
{
    Func<ISession> provider;
    public FuncDb1SessionProvider(Func<ISession> sessionProvider) {
        this.sessionProvier = provider; 
    }
    public ISession Session => provider();
}

And you can register such implementation in Simple Injector as follows:

var factory = new Configuration().Configure().SetProperty(
    "connection.connection_string",
    connectionStrings["db1"].ConnectionString)
    .BuildSessionFactory();

var session1Producer = Lifestyle.Scoped.CreateProducer<ISession>(
    factory.OpenSession, container);

container.RegisterSingleton<IDb1SessionProvider>(
    new FuncDb1SessionProvider(session1Producer.GetInstance));

What this code does is creating a scoped InstanceProducer for the db1 session. The scoped InstanceProducer will ensure only one instance of that session is created during a certain scope (usually a web request) and it will ensure that the ISession implementation is disposed (if it implements IDisposable). The call to InstanceProducer.GetInstance() is wrapped in the FuncDb1SessionProvider. This session provider will call forward the creation of the session to the wrapped delegate.

With this design you can let your application code depend on the IDb1SessionProvider and that code can use it without the need to dispose it. Every call to IDb1SessionProvider.Session within the same session will ensure you get the same session and Simple Injector guarantees disposal on the end of the request.

Upvotes: 2

Oskar Berggren
Oskar Berggren

Reputation: 5629

It looks like you have invented your own interface called ISessionFactory. Given that you are using NHibernate which also provides an interface under this name, I would argue that it's VERY unfortunate to use the same names in your own code. You should pick a different name for your own interface and class to avoid confusion.

As for the question itself, NHibernate's ISessionFactory.OpenSession() does exactly that. It will open and return a session. There is no basis to assume that it will do something magic with regards to reuse or scoping.

To have NHibernate assist with contextual sessions, you need to configure the proper "context provider" and use, among other things, ISessionFactory.GetCurrentSession(). See Contextual Sessions in the NHibernate reference.

Alternatively, you can manage the sessions using whatever you like, but then you must use that mechanism to retrieve the current session and not expect NHibernate to know about it.

Upvotes: 2

Related Questions