Skip to content

Commit

Permalink
#156 fixed Regression when creating nested loggers in reverse order
Browse files Browse the repository at this point in the history
  • Loading branch information
@FreeAndNil
FreeAndNil committed Jul 26, 2024
1 parent 48d0fad commit b5612e2
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 75 deletions.
125 changes: 74 additions & 51 deletions src/log4net.Tests/Hierarchy/HierarchyTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
using System;
using System.Xml;
using log4net.Config;
using log4net.Core;
using log4net.Repository;
using log4net.Repository.Hierarchy;
using log4net.Tests.Appender;
using NUnit.Framework;

Expand All @@ -39,19 +37,19 @@ public void SetRepositoryPropertiesInConfigFile()
// LOG4NET-53: Allow repository properties to be set in the config file
XmlDocument log4netConfig = new XmlDocument();
log4netConfig.LoadXml(@"
<log4net>
<property>
<key value=""two-plus-two"" />
<value value=""4"" />
</property>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
</log4net>");
<log4net>
<property>
<key value=""two-plus-two"" />
<value value=""4"" />
</property>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
</log4net>");

ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
Expand Down Expand Up @@ -101,18 +99,18 @@ public void LoggerNameCanConsistOfASingleDot()
{
XmlDocument log4netConfig = new XmlDocument();
log4netConfig.LoadXml(@"
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""."">
<level value=""WARN"" />
</logger>
</log4net>");
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""."">
<level value=""WARN"" />
</logger>
</log4net>");

ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
Expand All @@ -123,18 +121,18 @@ public void LoggerNameCanConsistOfASingleNonDot()
{
XmlDocument log4netConfig = new XmlDocument();
log4netConfig.LoadXml(@"
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""L"">
<level value=""WARN"" />
</logger>
</log4net>");
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""L"">
<level value=""WARN"" />
</logger>
</log4net>");

ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
Expand All @@ -145,21 +143,46 @@ public void LoggerNameCanContainSequenceOfDots()
{
XmlDocument log4netConfig = new XmlDocument();
log4netConfig.LoadXml(@"
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""L..M"">
<level value=""WARN"" />
</logger>
</log4net>");
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
<logger name=""L..M"">
<level value=""WARN"" />
</logger>
</log4net>");

ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
}

/// <summary>
/// https://github.com/apache/logging-log4net/issues/156
/// Regression: Creating nested loggers in reverse order fails in 3.0.0-preview.1
/// </summary>
[Test]
public void CreateNestedLoggersInReverseOrder()
{
XmlDocument log4netConfig = new XmlDocument();
log4netConfig.LoadXml(@"
<log4net>
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
<layout type=""log4net.Layout.SimpleLayout"" />
</appender>
<root>
<level value=""ALL"" />
<appender-ref ref=""StringAppender"" />
</root>
</log4net>");

ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
Assert.AreEqual("A.B.C", rep.GetLogger("A.B.C").Name);
Assert.AreEqual("A.B", rep.GetLogger("A.B").Name);
}
}
}
59 changes: 39 additions & 20 deletions src/log4net/Repository/Hierarchy/Hierarchy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ namespace log4net.Repository.Hierarchy
/// </remarks>
public class LoggerCreationEventArgs : EventArgs
{
/// <summary>
/// <summary>
/// Constructor
/// </summary>
/// <param name="log">The <see cref="Logger"/> that has been created.</param>
Expand Down Expand Up @@ -660,43 +660,62 @@ public Logger GetLogger(string name, ILoggerFactory factory)

var key = new LoggerKey(name);

// Synchronize to prevent write conflicts. Read conflicts (in
// GetEffectiveLevel() method) are possible only if variable
// assignments are non-atomic.
while (true)
{
if (TryCreateLogger(key, factory) is Logger result)
{
return result;
}
}
}

private Logger? TryCreateLogger(LoggerKey key, ILoggerFactory factory)
{
if (!m_ht.TryGetValue(key, out object? node))
{
return CreateLogger(null);
Logger newLogger = CreateLogger(key.Name);
node = m_ht.GetOrAdd(key, newLogger);
if (node == newLogger)
{
RegisterLogger(newLogger);
}
}

if (node is Logger nodeLogger)
if (node is Logger logger)
{
return nodeLogger;
return logger;
}

if (node is ProvisionNode nodeProvisionNode)
if (node is ProvisionNode provisionNode)
{
return CreateLogger(l => UpdateChildren(nodeProvisionNode, l));
Logger newLogger = CreateLogger(key.Name);
if (m_ht.TryUpdate(key, newLogger, node))
{
UpdateChildren(provisionNode, newLogger);
RegisterLogger(newLogger);
return newLogger;
}
return null;
}

// It should be impossible to arrive here but let's keep the compiler happy.
return null!;

Logger CreateLogger(Action<Logger>? extraInit)
Logger CreateLogger(string name)
{
// Use GetOrAdd in case the logger was added after checking above.
return (Logger)m_ht.GetOrAdd(key, _ =>
{
Logger logger = factory.CreateLogger(this, name);
logger.Hierarchy = this;
extraInit?.Invoke(logger);
UpdateParents(logger);
OnLoggerCreationEvent(logger);
return logger;
});
Logger result = factory.CreateLogger(this, name);
result.Hierarchy = this;
return result;
}

void RegisterLogger(Logger logger)
{
UpdateParents(logger);
OnLoggerCreationEvent(logger);
}
}


/// <summary>
/// Sends a logger creation event to all registered listeners
/// </summary>
Expand Down
12 changes: 8 additions & 4 deletions src/log4net/Repository/Hierarchy/LoggerKey.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace log4net.Repository.Hierarchy
/// </remarks>
/// <author>Nicko Cadell</author>
/// <author>Gert Driesen</author>
[DebuggerDisplay("{m_name}")]
[DebuggerDisplay("{Name}")]
internal readonly struct LoggerKey
{
/// <summary>
Expand All @@ -65,7 +65,7 @@ internal readonly struct LoggerKey
/// <param name="name">The name of the logger.</param>
internal LoggerKey(string name)
{
m_name = string.Intern(name);
Name = string.Intern(name);
m_hashCache = name.GetHashCode();
}

Expand All @@ -83,7 +83,11 @@ public override int GetHashCode()
return m_hashCache;
}

private readonly string m_name;
/// <summary>
/// Name of the Logger
/// </summary>
internal string Name { get; }

private readonly int m_hashCache;

public static Comparer ComparerInstance { get; } = new();
Expand All @@ -92,7 +96,7 @@ public sealed class Comparer : IEqualityComparer<LoggerKey>
{
public bool Equals(LoggerKey x, LoggerKey y)
{
return x.m_hashCache == y.m_hashCache && x.m_name == y.m_name;
return x.m_hashCache == y.m_hashCache && x.Name == y.Name;
}

public int GetHashCode(LoggerKey obj) => obj.m_hashCache;
Expand Down

0 comments on commit b5612e2

Please sign in to comment.