Monday, October 27, 2008

nHibernate Transaction & Session Mgmt

I've been using nHibernate for a couple of months now, and have been using Billy McCafferty's basic architecture with some of my own added spin. Today I noticed a strange behavior in nHibernate 2.0 which I've not fully explained or explored so bare with me.

I have an Organization object, and there's a screen where a user can modify the properties and save the object back to the database. I wanted to add additional error handling around the app to make it friendlier, so I removed one of my safeguards that let me generate an error (passing in a string larger then what the DB is configured for), and ran through my test. Essentially the page method doing the save looked like:

 protected void saveOrgInfo_Click(object sender, EventArgs e)
    {
        try
        {
            DoSave();
            this.saveStatus.Text = "Your changes have been saved.";
            this.saveStatus.Visible = true;
            this.saveStatus.CssClass = "successText";
        }
        catch (Exception ex)
        {
            CutterLogEntry.LogError("Error saving OrgInfo", ex);
 
            this.saveStatus.Text = "There was an error saving your changes. Please try again.";
            this.saveStatus.Visible = true;
            this.saveStatus.CssClass = "warnText";
        }
    }
 
    private void DoSave()
    {
        NHibernateSessionManager.Instance.BeginTransactionOn();
        MembershipService memService = new MembershipService();
        this.orgInfo.UpdateOrganizationInfo(Util.CurrentUser.Organization);
        memService.SaveOrganization(Util.CurrentUser.Organization);
        NHibernateSessionManager.Instance.CommitTransactionOn();
    }


One of my modifications was to add overloads to his methods which don't accept a path to the config file, and when this is used it defaults to the Web.Config / App.Config file.

What I found interesting was that I was being redirected to an error page, when based on this code my catch should have handled this. The call stack in the error page indicated that the error was originating in my httpModule which tries to close any open session.

When we tried to call Close on the session the nHibernateSessionManager also calls a flush() which will cause nHibernate to try and save the Organization entity again. The interesting thing here is if we look at the logic in the Commit method, when it fails the SessionManager tries to rollback the transaction:


        public void CommitTransactionOn(string sessionFactoryConfigPath) {
            ITransaction transaction = (ITransaction)ContextTransactions[sessionFactoryConfigPath];
 
            try {
                if (HasOpenTransactionOn(sessionFactoryConfigPath)) {
                    transaction.Commit();
                    ContextTransactions.Remove(sessionFactoryConfigPath);
                }
            }
            catch (HibernateException) {
                RollbackTransactionOn(sessionFactoryConfigPath);
                throw;
            }
        }



If we follow the source the Rollback will call the rollback transaction and then call the CloseSessionOn() method. The CloseSessionOn() method will attempt to flush and then close the session. The call to flush will generate an exception here. Because of this exception, the close method is never called.

So when the HttpModule kicks off it sees there is an active session, and then trying to close it calls a flush which generates the exception again. All in all this one exception was thrown three times so far.

I find it strange that nHibernate would try and save an object that was "Saved" within a transactional context. I would imagine that a Rollback should clear out all dirty entities; however, it doesn't. The quick fix is as I said to modify the Rollback method so that it closes (but not flush).

Enjoy

Josh

2 comments:

Anonymous said...

This is a great catch Berke! This has since been realized and corrected within the successor to this architecture, S#arp Architecture, found at http://code.google.com/p/sharp-architecture/. Not only is it promblematic to Flush at close, but also presumptuous to Flush upon close. It should be left up to the transaction, or to the developer to decide when this flush should occur.

Josh Berke said...

Thanks and I'm glad to hear you've resolved it, I will need to take a look at your new S#arp Architcture.

I'm debating now wether nHibernate should behave this when you flush after rolling back a transaction.