Skip to content

Commit

Permalink
Handle the case of concurrent session disposal
Browse files Browse the repository at this point in the history
  • Loading branch information
fredericDelaporte committed Feb 13, 2024
1 parent 5a1f3f4 commit cff5d98
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 18 deletions.
46 changes: 34 additions & 12 deletions src/NHibernate/Impl/AbstractSessionImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -410,16 +410,31 @@ public bool IsClosed
public IDisposable BeginProcess()
{
return _processHelper.BeginProcess(this);
}

/// <summary>
/// If not nested in a call to <c>BeginProcess</c> on this session, set its session id in context.
/// </summary>
/// <returns>
/// If not already processing, an object to dispose for restoring the previous session id.
/// Otherwise, <see langword="null" />.
/// </returns>
public IDisposable BeginContext()
}

/// <summary>
/// If not nested in another call to <c>BeginProcess</c> on this session, optionnaly check
/// and update the session status, then set its session id in context and flag it as processing.
/// </summary>
/// <param name="noCheckAndUpdate"><see langword="true" /> to initiate a processing without
/// checking and updating the session.</param>
/// <returns>
/// If not already processing, an object to dispose for signaling the end of the process.
/// Otherwise, <see langword="null" />.
/// </returns>
protected IDisposable BeginProcess(bool noCheckAndUpdate)
{
return _processHelper.BeginProcess(this, noCheckAndUpdate);
}

/// <summary>
/// If not nested in a call to <c>BeginProcess</c> on this session, set its session id in context.
/// </summary>
/// <returns>
/// If not already processing, an object to dispose for restoring the previous session id.
/// Otherwise, <see langword="null" />.
/// </returns>
public IDisposable BeginContext()
{
return _processHelper.Processing ? null : SessionIdLoggingContext.CreateOrNull(SessionId);
}
Expand All @@ -442,15 +457,22 @@ public ProcessHelper()

public bool Processing { get => _processing; }

public IDisposable BeginProcess(AbstractSessionImpl session)
public IDisposable BeginProcess(AbstractSessionImpl session, bool noCheckAndUpdate = false)
{
if (_processing)
return null;

try
{
_context = SessionIdLoggingContext.CreateOrNull(session.SessionId);
session.CheckAndUpdateSessionStatus();
if (noCheckAndUpdate)
{
session.TransactionContext?.Wait();
}
else
{
session.CheckAndUpdateSessionStatus();
}
_processing = true;
}
catch
Expand Down
9 changes: 4 additions & 5 deletions src/NHibernate/Impl/SessionImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1488,13 +1488,12 @@ public void Reconnect(DbConnection conn)
/// </summary>
public void Dispose()
{
using (BeginContext())
// Ensure we are not disposing concurrently to transaction completion, which would
// remove the transaction context.
using (BeginProcess(true))
{
log.Debug("[session-id={0}] running ISession.Dispose()", SessionId);
// Ensure we are not disposing concurrently to transaction completion, which would
// remove the context. (Do not store it into a local variable before the Wait.)
TransactionContext?.Wait();
// If the synchronization above is bugged and lets a race condition remaining, we may
// If the BeginProcess synchronization is bugged and lets a race condition remaining, we may
// blow here with a null ref exception after the null check. We could introduce
// a local variable for avoiding it, but that would turn a failure causing an exception
// into a failure causing a session and connection leak. So do not do it, better blow away
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,13 @@ protected virtual void CompleteTransaction(bool isCommitted)
if (!IsInActiveTransaction)
return;
Lock();
var isSessionProcessing = _session.IsProcessing();
// In case of a rollback due to a timeout, we may have the session disposal running concurrently
// to the transaction completion in a way our current locking mechanism cannot fully protect: the
// session disposal "BeginProcess" can go through the Wait before it is locked but flag the
// session as procesisng after the transaction completion has read it as not processing. To dodge
// that case, we consider the session as still processing initially regardless of its actual
// status in case of rollback.
var isSessionProcessing = !isCommitted || _session.IsProcessing();
try
{
// Allow transaction completed actions to run while others stay blocked.
Expand Down

0 comments on commit cff5d98

Please sign in to comment.