This is an automated email from the ASF dual-hosted git repository.
freeandnil pushed a commit to branch
Feature/157-fixed-Regression-when-creating-nested-loggers-in-reverse-order
in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
commit b5612e299721b108dfa54258cee26a8788193db5
Author: Jan Friedrich <freeand...@apache.org>
AuthorDate: Fri Jul 26 10:37:28 2024 +0200
#156 fixed Regression when creating nested loggers in reverse order
---
src/log4net.Tests/Hierarchy/HierarchyTest.cs | 125 +++++++++++++++-----------
src/log4net/Repository/Hierarchy/Hierarchy.cs | 59 +++++++-----
src/log4net/Repository/Hierarchy/LoggerKey.cs | 12 ++-
3 files changed, 121 insertions(+), 75 deletions(-)
diff --git a/src/log4net.Tests/Hierarchy/HierarchyTest.cs
b/src/log4net.Tests/Hierarchy/HierarchyTest.cs
index 7cd39ae3..68c9451e 100644
--- a/src/log4net.Tests/Hierarchy/HierarchyTest.cs
+++ b/src/log4net.Tests/Hierarchy/HierarchyTest.cs
@@ -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;
@@ -39,19 +37,19 @@ namespace log4net.Tests.Hierarchy
// 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"]!);
@@ -101,18 +99,18 @@ namespace log4net.Tests.Hierarchy
{
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"]!);
@@ -123,18 +121,18 @@ namespace log4net.Tests.Hierarchy
{
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"]!);
@@ -145,21 +143,46 @@ namespace log4net.Tests.Hierarchy
{
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);
+ }
}
}
diff --git a/src/log4net/Repository/Hierarchy/Hierarchy.cs
b/src/log4net/Repository/Hierarchy/Hierarchy.cs
index 8e7a9625..d63b6391 100644
--- a/src/log4net/Repository/Hierarchy/Hierarchy.cs
+++ b/src/log4net/Repository/Hierarchy/Hierarchy.cs
@@ -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>
@@ -660,43 +660,62 @@ namespace log4net.Repository.Hierarchy
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>
diff --git a/src/log4net/Repository/Hierarchy/LoggerKey.cs
b/src/log4net/Repository/Hierarchy/LoggerKey.cs
index aaf553d0..0acc21af 100644
--- a/src/log4net/Repository/Hierarchy/LoggerKey.cs
+++ b/src/log4net/Repository/Hierarchy/LoggerKey.cs
@@ -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>
@@ -65,7 +65,7 @@ namespace log4net.Repository.Hierarchy
/// <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();
}
@@ -83,7 +83,11 @@ namespace log4net.Repository.Hierarchy
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();
@@ -92,7 +96,7 @@ namespace log4net.Repository.Hierarchy
{
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;
