Skip to content
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

Improved re-entrancy Scope Lock for Session Value Layout Render #850

Merged
merged 18 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
42 changes: 22 additions & 20 deletions src/Shared/Internal/RendererReEntrantManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,39 @@ namespace NLog.Web.Internal
/// Since NET 35 does not support AsyncLocal or even ThreadLocal
/// a different technique must be used for that platform
/// </summary>
internal struct RendererReEntrantManager : IDisposable
internal readonly struct RendererReEntrantManager : IDisposable
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
{
internal RendererReEntrantManager(HttpContextBase context)
{
_httpContext = context;

// The line below is required, because this is a struct, otherwise we get CS0188 compiler error
// which is 'The 'this' object cannot be used before all of its fields have been assigned.'
_obtainedLock = false;
TryGetLock();

_obtainedLock = TryGetLock();
}

private readonly HttpContextBase _httpContext;

// Need to track if we were successful in the lock
// If we were not, we should not unlock in the dispose code
private readonly bool _obtainedLock;

internal bool IsLockAcquired => _obtainedLock;

private bool TryGetLock()
{
// If already locked, return false
if (IsLocked())
{
return false;
}
// Get the lock
Lock();
return true;
}

#if NET35
private static readonly object ReEntrantLock = new object();

Expand Down Expand Up @@ -66,24 +86,6 @@ private void Unlock()
}
#endif

// Need to track if we were successful in the lock
// If we were not, we should not unlock in the dispose code
private bool _obtainedLock;


private void TryGetLock()
{
// If already locked, return false
if (IsLocked())
{
return;
}
// Get the lock
Lock();
// Mark that we locked it, not another instance locked it
_obtainedLock = true;
}

private void DisposalImpl()
{
// Only unlock if we were the ones who locked it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ protected override void DoAppend(StringBuilder builder, LogEventInfo logEvent)
{
if (!reEntry.IsLockAcquired)
{
InternalLogger.Error(
$"Reentrant log event detected. Logging when inside the scope of another log event can cause a StackOverflowException. LogEventInfo.Message:{logEvent.Message}");
InternalLogger.Debug(
$"Reentrant log event detected. Logging when inside the scope of another log event can cause a StackOverflowException.");
snakefoot marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand Down