Add new session context implementation for TransactionScope

[ah1508]

It should be possible to bind a session lifecycle to a System.Transactions.TransactionScope.

An implementation of NHibernate.Context.ICurrentSessionContext would work as follow (method GetCurrentSession).

If first call for the current transaction

1. opens a session (sessionFactory.OpenSession())
2. register a callback for the System.Transactions.Transaction.Current.TransactionCompleted event, which flush the session (if transaction status is Committed) and close it.
3. add the session to a IDictionary<string, ISession> where the key is the transaction identifier (Transaction.Current.TransactionInformation.LocalIdentifier) + sessionfactory hashcode and the value is the session newly created.

If not first call for the current transaction : return the session found in the IDictionary<string, ISession>

I found information about ISession and transaction scope synchronization, but I never got the expected result. I finally wrote an implementation of ICurrentSessionContext which works as described above. Is there something wrong in the implementation described here above ?

[fredericDelaporte]

About 2.,

register a callback for the System.Transactions.Transaction.Current.TransactionCompleted event, which flush the session (if transaction status is Committed) and close it.

This means, at "best", flushing the session out of the transaction. At worst, it will fail, the transaction being in a state no more allowing any queries while the session connection would still be enlisted into it.
In case only the "best" occurs, it does not look to me as an interesting pattern.

The worst is more likely to occur randomly when the scope was distributed. In such case, the transaction completion event is run on its own thread, concurrently to the second phase of the connection (and of other resources enlisted in the transaction, and even concurrently to the code following the scope disposal), which creates race conditions. (See this comment, and run these tests to see how things can get all mixed and entangled.)

Moreover this 2. adds a new case of session being closed from system transactions events, while we are moving away from doing such things. It tends to create connection pool corruption and random failures.

If you want more details about those potential troubles, you can find some long read on NH-4011.

About 3.,

the key is the transaction identifier (Transaction.Current.TransactionInformation.LocalIdentifier) + sessionfactory hashcode

Better not use hashcode for this. They are not unique keys. Even if unlikely, two different session factories may have the same hashcode. A ConcurrentDictionary<ISessionFactory, ConcurrentDictionary<Guid, ISession>> could be used instead. Otherwise if you would rather stick to using a composed key, you may cast the session factory to NHibernate.Impl.SessionFactoryImpl and use its Uuid property.

Update: I forgot, but also better use ConcurrentDictionary instead of IDictionary. It looks very likely to me their usages will have to be thread safe.

[ah1508]

My bad for 2., flushing the session before transaction completion is already done by Nhibernate and doing it again after commit does not make sense.

However session closing remains unclear, if I don't close then session when transaction is complete I don't understand when it should be done and by who. #NH-4011 helps to understand the problem but I could not find how session closing is handled.

Thanks for pointing the the SessionFactoryImpl uuid.

[fredericDelaporte]

My bad for 2., flushing the session before transaction completion is already done by Nhibernate and doing it again after commit does not make sense.

This feature is the root cause of the many issues related to NH-4011, and it was even considered to remove it. Instead, it is advised to not rely on it by explicitly flushing the session before scope completion. And then, disable it if possible, by setting the transaction.use_connection_on_system_prepare setting to false.

However session closing remains unclear, if I don't close then session when transaction is complete I don't understand when it should be done and by who.

This all depends on the application. After all, it must have some place to open it, isn't it? Why couldn't it have somewhere to close it?
By the way, session lifespan does not have to match transaction lifespan. It is not unusual to have the session life spanning many transactions. For a web application or a web API, an usual practice is to close the session at the end of the request, if one was opened for the request. For a desktop application, session could be bound to each user action or event, and closed at their end.

There is also a special case where the session closing is done by NHibernate at the end of the transaction scope: when the user has disposed the session inside the scope, with transaction.use_connection_on_system_prepare set to true. In such case, the session is not actually disposed of until the end of the scope. But that is the case which causes the worst issues, as explained in NH-4011.

[ah1508]

Maybe it is because of my Java and Hibernate experience, but session lifespan == transaction lifespan is a common pattern that makes sense. However keeping the session open for the whole http request can be useful, especially useful for lazy loading in the view (open session in view pattern) but can also lead to n+1 select. In Java this behavior is optional.

I don't want my application code to be responsible for opening or closing the session. In many applications calls to service layer are surrounded by a transaction but the service layer is not even aware of the hibernate sessions and should not be responsible for opening it or closing it.

DispatchProxy helps to hide these transaction and sesssion lifecycle management, a proxy in front of service components can begin the transaction, open the session, invoke the requested method, flush the session (unless an exception occured) and close it (or not, cf. open session in view pattern) and commit the transaction (uneless an exception occured). But creating a such infrastructure only to get a proper separation between business code and session lifecycle management the first thing you want to do when you start with Nhibernate (it is not a criticism against NHibernate).

[fredericDelaporte]

The point is, using the transaction completed event (or a resource enlistment) for closing the session cannot be reliable and will create issues in a number of cases. So I do not think adding a session context closing the session from transaction scope completion should be done.

It may work only when the scope is not distributed. In such case, the transaction completed events seem to always be executed last, after all second phases of enlisted resources. So the session closing, causing a connection close, should not interfere with the connection second phase.
But if the scope is distributed, the transaction completed event will run concurrently to second phases of enlisted resources. Failures will randomly occur due to the connection getting sometimes closed while it was still executing its second phase. This is not acceptable, especially since some connections seems to always cause the scope to go distributed, like OdbcConnection, HanaConnection, AseConnection, ...

So if your application usage pattern can guarantee its scopes will never go distributed, such a session context could work for you. But better keep it a custom session context of your application. A context provided by NHibernate would have to support broader cases, and it will not be doable reliably.

This is not a NHibernate limitation, this is due to how distributed scopes behave. They provide no guarantee that the transaction completed event will run last and not concurrently to second phases of enlisted resources.

[ah1508]

I agree with you for distributed cases, but not every application has distributed transactions so I think it can make sense not only for my use case. 1 session per request, or per thread are two options, 1 per transaction could be another one. ICurrentSessionContext opens the way to several implementations, all opinionated with pros, cons and restrictions.

Keeping the transaction scope local is a reasonable restriction, but if the connection causes the scope to go distributed it is another problem, not even related with NHibernate. You say that It happens with Odbc, SAP and Sybase, but does that occurs with the most used databases (let's say Sql Server, Postgresql, MySQL and Oracle) ?

[ah1508]

In addition to what has already been written, more than half ot the built-in contextual session implementations are less relevant now : thread_static is not appropriate for async operations while wcf_operation and call only work for .NET framework. This leaves async_local and web for modern applications.

Since a TransactionScope contextual session management is not planed yet, I wrote my own.

It works well on .NET 5 with postgresql, sql server and mysql ; transaction.use_connection_on_system_prepare is set to false. Do you think I missed something ?

.NET Core and .NET 5 don't support distributed transactions, which is fine. But postgresql needs max_prepared_transactions > 0 (probably related to npgsql/npgsql#2155)

class TransactionScopeSessionContext : NHibernate.Context.ICurrentSessionContext
{
    private readonly ISessionFactory sessionFactory;
    private readonly INHibernateLogger logger;
    private readonly Dictionary<string, ISession> activeSessions = new();

    public TransactionScopeSessionContext(ISessionFactory sessionFactory)
    {
        this.sessionFactory = sessionFactory;
        this.logger = NHibernateLogger.For(GetType());
    }

    ISession NHibernate.Context.ICurrentSessionContext.CurrentSession()
    {
        Transaction? tx = System.Transactions.Transaction.Current;
        if (tx == null)
        {
            throw new InvalidOperationException("cannot manage a session outside a transaction");
        }

        string txid = tx.TransactionInformation.LocalIdentifier;

        logger.Debug("received a call to GetCurrentSession() in transaction {0}", txid);

        if (activeSessions.TryGetValue(txid, out ISession? currentSession))
        {
            logger.Debug("reuse session {0}", currentSession.GetSessionImplementation().SessionId);
            return currentSession;
        }
        else
        {
            logger.Debug("first call to GetCurrentSession() in this transaction, open a new session");
            ISession session = this.sessionFactory.OpenSession();
            Guid sessionId = session.GetSessionImplementation().SessionId;
            logger.Debug("opened session {0}", sessionId);
            logger.Debug("register session {0} for synchronization with transaction {1}", sessionId, txid);
            tx.EnlistVolatile(new FlushSessionOnPrepare(session, logger), EnlistmentOptions.EnlistDuringPrepareRequired);
            
            tx.TransactionCompleted += (x, y) =>
            {
                logger.Debug("close session {0}", sessionId);
                session.Close();
                logger.Debug("session{0} closed", sessionId);
                logger.Debug("remove session {0} from active sessions", sessionId);
                activeSessions.Remove(txid);
            };
            logger.Debug("add session {0} to active sessions under the key {1}", sessionId, txid);
            activeSessions.Add(txid, session);
            return session;
        }
    }
}

class FlushSessionOnPrepare : IEnlistmentNotification
{
    private readonly ISession session;
    private readonly INHibernateLogger logger;

    public FlushSessionOnPrepare(ISession session, INHibernateLogger logger) => (this.session, this.logger) = (session, logger);

    public void Prepare(PreparingEnlistment preparingEnlistment)
    {
        if(session.FlushMode == FlushMode.Always || session.FlushMode == FlushMode.Commit || session.FlushMode == FlushMode.Auto)
        {
            Guid sessionid = session.GetSessionImplementation().SessionId;
            logger.Info("flush session {0}", sessionid);
            session.Flush();
            logger.Info("session {0} flushed", sessionid);
        }
        preparingEnlistment.Prepared();
    }

    public void Commit(Enlistment enlistment) => enlistment.Done();
        
    public void InDoubt(Enlistment enlistment) => enlistment.Done();

    public void Rollback(Enlistment enlistment) => enlistment.Done();
}