Skip to content

Add GetOrBuild on the session factory factory #3668

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3657/Entity.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System;

namespace NHibernate.Test.NHSpecificTest.GH3657
{
class Entity
{
public virtual Guid Id { get; set; }
public virtual string Name { get; set; }
}
}
54 changes: 54 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3657/Fixture.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
using log4net;
using NHibernate.Cfg;
using NHibernate.Impl;
using NUnit.Framework;

namespace NHibernate.Test.NHSpecificTest.GH3657
{
[TestFixture]
public class Fixture
{
private static readonly ILog _log = LogManager.GetLogger(typeof(Fixture));
private const string TestSessionFactoryName = "TestName";

private Configuration _cfg;
private ISessionFactory _builtSessionFactory;

[OneTimeSetUp]
public void TestFixtureSetUp()
{
_cfg = TestConfigurationHelper.GetDefaultConfiguration();
var type = GetType();
_cfg.AddResource(type.Namespace + ".Mappings.hbm.xml", type.Assembly);
_cfg.SetProperty(Environment.SessionFactoryName, TestSessionFactoryName);
}

[TearDown]
public void TearDown()
{
_builtSessionFactory?.Dispose();
_builtSessionFactory = null;
}

private ISessionFactory SessionFactoryBuilder()
{
Assert.That(_builtSessionFactory, Is.Null, "SessionFactory was already built");

_builtSessionFactory = _cfg.BuildSessionFactory();
_log.Info("Successfully built session factory");

return _builtSessionFactory;
}

[Test]
public void GetOrAddTwice()
{
var factory = SessionFactoryObjectFactory.GetOrBuildNamedInstance(TestSessionFactoryName, SessionFactoryBuilder);
Assert.That(factory, Is.Not.Null, "Failed to get the factory once");

var factory2 = SessionFactoryObjectFactory.GetOrBuildNamedInstance(TestSessionFactoryName, SessionFactoryBuilder);
Assert.That(factory2, Is.Not.Null, "Failed to get the factory twice");
Assert.That(factory, Is.SameAs(factory2), "The two factories should be the same");
}
}
}
10 changes: 10 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3657/Mappings.hbm.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="utf-8" ?>
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
namespace="NHibernate.Test.NHSpecificTest.GH3657">

<class name="Entity">
<id name="Id" generator="guid.comb"/>
<property name="Name"/>
</class>

</hibernate-mapping>
37 changes: 34 additions & 3 deletions src/NHibernate/Impl/SessionFactoryObjectFactory.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;

Expand Down Expand Up @@ -78,16 +79,46 @@ public static void RemoveInstance(string uid, string name, IDictionary<string, s
}

/// <summary>
/// Returns a Named Instance of the SessionFactory from the local "cache" identified by name.
/// Get an instance of the SessionFactory from the local "cache" identified by name if it
/// exists, otherwise run the provided factory and return its result.
/// </summary>
/// <param name="name">The name of the ISessionFactory.</param>
/// <param name="instanceBuilder">The ISessionFactory factory to use if the instance is not
/// found.</param>
/// <returns>An instantiated ISessionFactory.</returns>
/// <remarks>
/// <para>It is the caller responsibility to ensure <paramref name="instanceBuilder"/>
/// will add and yield a session factory of the requested <paramref name="name"/>.</para>
/// <para>If the session factory instantiation is performed concurrently outside of a
/// <c>GetOrAddNamedInstance</c> call, this method may yield an instance of it still being
/// built, which may lead to threading issues.</para>
/// </remarks>
[MethodImpl(MethodImplOptions.Synchronized)]
public static ISessionFactory GetOrBuildNamedInstance(string name, Func<ISessionFactory> instanceBuilder)
Copy link
Member Author

@fredericDelaporte fredericDelaporte Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow threadsafe use of the factory, a get or build is needed. Some applications do use it in a multi-threads context. They were always exposed to threading bugs due to the get being able of yielding a not yet fully built instance in such case, but this was worsen by #2190. With this GetOrBuild, provided the application only build factories through it, fully built instances will be yielded.

{
if (instanceBuilder == null)
throw new ArgumentNullException(nameof(instanceBuilder));

if (NamedInstances.TryGetValue(name, out var factory))
return factory;
return instanceBuilder();
}

/// <summary>
/// Returns a Named Instance of the SessionFactory from the local "cache" identified by name.
/// </summary>
/// <param name="name">The name of the ISessionFactory.</param>
/// <returns>An ISessionFactory if found, <see langword="null" /> otherwise.</returns>
/// <remarks>If the session factory instantiation is performed concurrently, this method
/// may yield an instance of it still being built, which may lead to threading issues.
/// Use <see cref="GetOrBuildNamedInstance(string, Func{ISessionFactory})" /> to get or
/// built the session factory in such case.</remarks>
[MethodImpl(MethodImplOptions.Synchronized)]
public static ISessionFactory GetNamedInstance(string name)
{
log.Debug("lookup: name={0}", name);
ISessionFactory factory;
bool found=NamedInstances.TryGetValue(name, out factory);
bool found = NamedInstances.TryGetValue(name, out factory);
if (!found)
{
log.Warn("Not found: {0}", name);
Expand All @@ -99,7 +130,7 @@ public static ISessionFactory GetNamedInstance(string name)
/// Returns an Instance of the SessionFactory from the local "cache" identified by UUID.
/// </summary>
/// <param name="uid">The identifier of the ISessionFactory.</param>
/// <returns>An instantiated ISessionFactory.</returns>
/// <returns>An ISessionFactory if found, <see langword="null" /> otherwise.</returns>
[MethodImpl(MethodImplOptions.Synchronized)]
public static ISessionFactory GetInstance(string uid)
{
Expand Down
Loading