In #2190 the time that the SessionFactoryObjectFactory cache for an instance was moved up before the SessionFactoryImpl is fully constructed. This can mean that during the ISessionFactory constructor, SessionFactoryObjectFactory.GetNamedInstance can return a partially constructed ISessionFactory

In the case where application start wasn't the right time to create the ISessionFactory and double checked locking was recommended. An example:

private static ISessionFactory GetSessionFactory(sessionFactoryName)
{
    var sessionFactory = SessionFactoryObjectFactory.GetNamedInstance(sessionFactoryName);
    if (sessionFactory == null)
    {
        lock (_sessionFactoryApplicationLock)
        {
            sessionFactory = SessionFactoryObjectFactory.GetNamedInstance(sessionFactoryName);
            if (sessionFactory == null)
            {
                sessionFactory = GetConfig(sessionFactoryName).BuildSessionFactory();
            }
        }
    }
    return sessionFactory;
}

When multiple threads hit this at the same time, please imagine:
Thread 1 comes in, SessionFactoryObjectFactory.GetNamedInstance returns null.
Thread 1 acquires the lock the second SessionFactoryObjectFactory.GetNamedInstance returns null.
Thread 1 starts building the session factory. Early in the ISessionFactory the cache is populated with a not yet built SessionFactory
Thread 2 calls SessionFactoryObjectFactory.GetNamedInstance and gets the partially built ISessionFactory
Bad behavior: Thread 1 is writing to the various dictionaries while thread 2 is reading them. We've seen entityPersisters not get fully populated(which populates one at a time in a loop on a normal dictionary) while classMetadata is populated(which populates a temporary dictionary and then uses a readonly dictionary). Dictionaries are thread safe if multiple threads are reading from them at once, but if any writing is going on at that time, the behavior is undefined and will break

Suggested improvement
The ISessionFactory is meant to be an unchanging, thread safe(in use, but not creation) object. The various non-readonly dictionaries could be changed to readonly dictionaries to have better safety(although this doesn't fully fix the issue - it would make the behavior more deterministic, the dictionary either exists with all the values or is null). This would help people who have older systems that followed best practice recommendations of the time not get session factories that are broken for their lifetime, but only for the critical point before the readonly dictionaries are populated.

It also probably makes sense to make SessionFactoryObjectFactory's dictionaries be thread safe. Pretty low cost to the change here and it would help an issue in the same vein with readers and writers both using the dictionaries at the same time.

On the side of the code using SessionFactoryObjectFactory.GetNamedInstance, moving the lock to cover the reader will(I believe) fix this as well. But the odd partial build behavior can be fixed here, at least improving the situation. Removing the use of SessionFactoryObjectFactory and handling caching outside NH for the ISessionFactory also is an option.

While it is important that one(and only one) ISessionFactory be built at a time, this behavior causes a very hard to debug and understand issue, where the object gets partially built successfully and partially not built, causing very odd errors that would confuse people who have historically used the current SessionFactoryObjectFactory implementation based on historical best practices.

Some historical context:
NH requires that the SessionFactory is only built once per NamedInstance. In some cases, application start is a tough time to do this(for example in a multi-tenant system with multiple databases sharing the same schema).
Some context here and in the links contained(including the book "Hibernate in Action (In Action series) [ILLUSTRATED] (Paperback)")
https://web.archive.org/web/20071011005024/http://www.codeproject.com/aspnet/NHibernateBestPractices.asp