Micah
Micah

Reputation: 116100

How to properly use a NHibernate ISession object - Session Is Closed! errors

I'm running into issues with my ISessions in NHibernate. I keep getting "Session Closed!" errors. Can some one please show me the correct pattern including a definition of the following methods and when to use each:

ISession.Close()
ISession.Dispose()
ISession.Disconnect()

Here's my problem. I have a callback setup to fire off a process that awards badges to players every couple of minutes. However I keep getting "Session Closed!" errors or errors about not being able to associate collections.

Here's my Repository:

public class NHibernateRepository : IRepository
{
#region Fields

private ISession _session;
private readonly ISessionFactory _sessionFactory;
#endregion

#region Constructors

public NHibernateRepository(ISessionFactory sessionFactory)
{
    _sessionFactory = sessionFactory;
}

#endregion

#region IRepository Implementation

public ISession OpenSession()
{
    _session = _sessionFactory.OpenSession();
    return _session;
}

public IQueryable<TModel> All<TModel>()
{
    return _session.Linq<TModel>();
}

public void Save<TModel>(TModel model)
{
    _session.Save(model);
}
public void Update<TModel>(TModel model)
{
    _session.Update(model);
}
public void Delete<TModel>(TModel model)
{
    _session.Delete(model);
}

public ITransaction BeginTransaction()
{
    return _session.BeginTransaction();
}
public void Flush()
{
    _session.Flush();
}
#endregion

}

Here's my usage. The repository is getting injected via Structure Map

private Object _awardBadgesLock = new object(); //In case the callback happens again before the previous one completes

public void AwardBadges()
{

    lock (_awardBadgesLock)
    {
        using(session = _repository.OpenSession())
        {
            foreach (var user in _repository.All<User>().ToList())
            {
                var userPuzzles = _repository.All<Puzzle>().ByUser(user.Id).ToList();
                var userVotes = _repository.All<Vote>().Where(x => x.UserId == user.Id).ToList();
                var userSolutions = _repository.All<Solution>().ByUser(user.Id).ToList().Where(x => !userPuzzles.Select(y => y.Id).Contains(x.PuzzleId));
                var ledPuzzles = GetPuzzlesLedByUser(user.Id);

                AwardPlayerBadge(user, userSolutions);
                AwardCriticBadge(user, userVotes);
                AwardCreatorBadge(user, userPuzzles);
                AwardRidlerBadge(user, userPuzzles);
                AwardSupporterBadge(user, userVotes);
                AwardPopularBadge(user, userPuzzles);
                AwardNotableBadge(user, userPuzzles);
                AwardFamousBadge(user, userPuzzles);
                AwardLeaderBadge(user, ledPuzzles);

                using (var tx = _repository.BeginTransaction())
                {
                    _repository.Update(user);
                    tx.Commit();
                }
            }
        }
    }

}

Upvotes: 11

Views: 27501

Answers (4)

M.G.E
M.G.E

Reputation: 371

About the problem, your method of locking is right as long as you dispose the session but probably the bug lies under another part of your codes. by the way about the design, it is better that you pass the session variable to repositories because of unit of work implementation of the session and aggregate root's transaction like this:

using (ISession session = SessionFactory.OpenSession())
{
    Repository1 rep1 = new Repository1(session);
    Repository2 rep1 = new Repository2(session);
    Repository3 rep1 = new Repository3(session);

    // some logics

    using (var tx = session.BeginTransaction())
        tx.Commit();
}

. . .

Upvotes: 0

Micah
Micah

Reputation: 116100

The issue lies in the fact the ISession is not thread-safe. There were multiple methods being fired on separate threads that all created an instance of ISession. The issue was really with the fact that they all shared the same SessionFactory. Image both of these methods are fired off on separate threads:

ISessionFactory _sessionFactory;

void MethodOne()
{
   using(ISession session = _sessionFactory.OpenSession()) 
   {
       //Do something with really quick with the session
       //Then dispose of it
   }
}

void MethodTwo()
{
   //OpenSession() actually returns the same instance used in the 
   //previous method that has already disposed of the object;
   using(ISession session = _sessionFactory.OpenSession()) 
   {
       //Do something with a session that has already been disposed
       //throws errors

   }
}

How I fixed it was basically ditching NHIbernate in these scenarios and called stored procs instead. I think it turned out to be more performant in my situation anyway.

Upvotes: 1

Mouk
Mouk

Reputation: 1737

I advice you to read the documentation of ISession on https://nhibernate.svn.sourceforge.net/svnroot/nhibernate/trunk/nhibernate/src/NHibernate/ISession.cs

Anyway the proper way to clean up when you are finished with the session is to dispose it (or better, surround the usage with using statement). In this case, "using" closes the session and suppresses the finalizer, i.e. it prevents the session object from unnecessarily surviving the next garbage collecting and saves the memory.

If the connection is already closed, disposing it will not throw an exception. On the other hand, closing after disposing (or after closing) throws an exception.

The documentation recommends calling disconnect instead of closing, because this releases the connection to the connection pool. You should call Reconnect before using a disconnected session.

For my needs, I always use "using" which calls Dispose and have never used the othe two functions.

Upvotes: 7

Ayende Rahien
Ayende Rahien

Reputation: 22956

You should always use session.Dispose(); The other are for very strange occurances

Upvotes: 14

Related Questions