Skip to content

Commit 22a3b1d

Browse files
FreeAndNilFreeAndNil
committed
#156 fixed Regression when creating nested loggers in reverse order
1 parent 48d0fad commit 22a3b1d

File tree

4 files changed

+128
-76
lines changed

4 files changed

+128
-76
lines changed

‎src/log4net.Tests/Hierarchy/HierarchyTest.cs‎

Lines changed: 74 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@
2222
using System;
2323
using System.Xml;
2424
using log4net.Config;
25-
using log4net.Core;
2625
using log4net.Repository;
27-
using log4net.Repository.Hierarchy;
2826
using log4net.Tests.Appender;
2927
using NUnit.Framework;
3028

@@ -39,19 +37,19 @@ public void SetRepositoryPropertiesInConfigFile()
3937
// LOG4NET-53: Allow repository properties to be set in the config file
4038
XmlDocument log4netConfig = new XmlDocument();
4139
log4netConfig.LoadXml(@"
42-
<log4net>
43-
<property>
44-
<key value=""two-plus-two"" />
45-
<value value=""4"" />
46-
</property>
47-
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
48-
<layout type=""log4net.Layout.SimpleLayout"" />
49-
</appender>
50-
<root>
51-
<level value=""ALL"" />
52-
<appender-ref ref=""StringAppender"" />
53-
</root>
54-
</log4net>");
40+
<log4net>
41+
<property>
42+
<key value=""two-plus-two"" />
43+
<value value=""4"" />
44+
</property>
45+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
46+
<layout type=""log4net.Layout.SimpleLayout"" />
47+
</appender>
48+
<root>
49+
<level value=""ALL"" />
50+
<appender-ref ref=""StringAppender"" />
51+
</root>
52+
</log4net>");
5553

5654
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
5755
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
@@ -101,18 +99,18 @@ public void LoggerNameCanConsistOfASingleDot()
10199
{
102100
XmlDocument log4netConfig = new XmlDocument();
103101
log4netConfig.LoadXml(@"
104-
<log4net>
105-
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
106-
<layout type=""log4net.Layout.SimpleLayout"" />
107-
</appender>
108-
<root>
109-
<level value=""ALL"" />
110-
<appender-ref ref=""StringAppender"" />
111-
</root>
112-
<logger name=""."">
113-
<level value=""WARN"" />
114-
</logger>
115-
</log4net>");
102+
<log4net>
103+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
104+
<layout type=""log4net.Layout.SimpleLayout"" />
105+
</appender>
106+
<root>
107+
<level value=""ALL"" />
108+
<appender-ref ref=""StringAppender"" />
109+
</root>
110+
<logger name=""."">
111+
<level value=""WARN"" />
112+
</logger>
113+
</log4net>");
116114

117115
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
118116
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
@@ -123,18 +121,18 @@ public void LoggerNameCanConsistOfASingleNonDot()
123121
{
124122
XmlDocument log4netConfig = new XmlDocument();
125123
log4netConfig.LoadXml(@"
126-
<log4net>
127-
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
128-
<layout type=""log4net.Layout.SimpleLayout"" />
129-
</appender>
130-
<root>
131-
<level value=""ALL"" />
132-
<appender-ref ref=""StringAppender"" />
133-
</root>
134-
<logger name=""L"">
135-
<level value=""WARN"" />
136-
</logger>
137-
</log4net>");
124+
<log4net>
125+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
126+
<layout type=""log4net.Layout.SimpleLayout"" />
127+
</appender>
128+
<root>
129+
<level value=""ALL"" />
130+
<appender-ref ref=""StringAppender"" />
131+
</root>
132+
<logger name=""L"">
133+
<level value=""WARN"" />
134+
</logger>
135+
</log4net>");
138136

139137
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
140138
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
@@ -145,21 +143,46 @@ public void LoggerNameCanContainSequenceOfDots()
145143
{
146144
XmlDocument log4netConfig = new XmlDocument();
147145
log4netConfig.LoadXml(@"
148-
<log4net>
149-
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
150-
<layout type=""log4net.Layout.SimpleLayout"" />
151-
</appender>
152-
<root>
153-
<level value=""ALL"" />
154-
<appender-ref ref=""StringAppender"" />
155-
</root>
156-
<logger name=""L..M"">
157-
<level value=""WARN"" />
158-
</logger>
159-
</log4net>");
146+
<log4net>
147+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
148+
<layout type=""log4net.Layout.SimpleLayout"" />
149+
</appender>
150+
<root>
151+
<level value=""ALL"" />
152+
<appender-ref ref=""StringAppender"" />
153+
</root>
154+
<logger name=""L..M"">
155+
<level value=""WARN"" />
156+
</logger>
157+
</log4net>");
160158

161159
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
162160
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
163161
}
162+
163+
/// <summary>
164+
/// https://github.com/apache/logging-log4net/issues/156
165+
/// Regression: Creating nested loggers in reverse order fails in 3.0.0-preview.1
166+
/// </summary>
167+
[Test]
168+
public void CreateNestedLoggersInReverseOrder()
169+
{
170+
XmlDocument log4netConfig = new XmlDocument();
171+
log4netConfig.LoadXml(@"
172+
<log4net>
173+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
174+
<layout type=""log4net.Layout.SimpleLayout"" />
175+
</appender>
176+
<root>
177+
<level value=""ALL"" />
178+
<appender-ref ref=""StringAppender"" />
179+
</root>
180+
</log4net>");
181+
182+
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
183+
XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
184+
Assert.AreEqual("A.B.C", rep.GetLogger("A.B.C").Name);
185+
Assert.AreEqual("A.B", rep.GetLogger("A.B").Name);
186+
}
164187
}
165188
}

‎src/log4net/Repository/Hierarchy/Hierarchy.cs‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ namespace log4net.Repository.Hierarchy
5050
/// </remarks>
5151
public class LoggerCreationEventArgs : EventArgs
5252
{
53-
/// <summary>
53+
/// <summary>
5454
/// Constructor
5555
/// </summary>
5656
/// <param name="log">The <see cref="Logger"/> that has been created.</param>
@@ -660,43 +660,65 @@ public Logger GetLogger(string name, ILoggerFactory factory)
660660

661661
var key = new LoggerKey(name);
662662

663-
// Synchronize to prevent write conflicts. Read conflicts (in
664-
// GetEffectiveLevel() method) are possible only if variable
665-
// assignments are non-atomic.
663+
const int maxRetries = 5;
664+
for (int i = 0; i < maxRetries; i++)
665+
{
666+
if (TryCreateLogger(key, factory) is Logger result)
667+
{
668+
return result;
669+
}
670+
}
671+
throw new InvalidOperationException(
672+
$"GetLogger failed, because possibly too many threads are messing with creating the logger {name}!");
673+
}
666674

675+
private Logger? TryCreateLogger(LoggerKey key, ILoggerFactory factory)
676+
{
667677
if (!m_ht.TryGetValue(key, out object? node))
668678
{
669-
return CreateLogger(null);
679+
Logger newLogger = CreateLogger(key.Name);
680+
node = m_ht.GetOrAdd(key, newLogger);
681+
if (node == newLogger)
682+
{
683+
RegisterLogger(newLogger);
684+
}
670685
}
671686

672-
if (node is Logger nodeLogger)
687+
if (node is Logger logger)
673688
{
674-
return nodeLogger;
689+
return logger;
675690
}
676691

677-
if (node is ProvisionNode nodeProvisionNode)
692+
if (node is ProvisionNode provisionNode)
678693
{
679-
return CreateLogger(l => UpdateChildren(nodeProvisionNode, l));
694+
Logger newLogger = CreateLogger(key.Name);
695+
if (m_ht.TryUpdate(key, newLogger, node))
696+
{
697+
UpdateChildren(provisionNode, newLogger);
698+
RegisterLogger(newLogger);
699+
return newLogger;
700+
}
701+
return null;
680702
}
681703

682704
// It should be impossible to arrive here but let's keep the compiler happy.
683-
return null!;
705+
throw new InvalidOperationException("TryCreateLogger failed, because a node is neither a Logger nor a ProvisionNode!");
684706

685-
Logger CreateLogger(Action<Logger>? extraInit)
707+
Logger CreateLogger(string name)
686708
{
687-
// Use GetOrAdd in case the logger was added after checking above.
688-
return (Logger)m_ht.GetOrAdd(key, _ =>
689-
{
690-
Logger logger = factory.CreateLogger(this, name);
691-
logger.Hierarchy = this;
692-
extraInit?.Invoke(logger);
693-
UpdateParents(logger);
694-
OnLoggerCreationEvent(logger);
695-
return logger;
696-
});
709+
Logger result = factory.CreateLogger(this, name);
710+
result.Hierarchy = this;
711+
return result;
712+
}
713+
714+
void RegisterLogger(Logger logger)
715+
{
716+
UpdateParents(logger);
717+
OnLoggerCreationEvent(logger);
697718
}
698719
}
699720

721+
700722
/// <summary>
701723
/// Sends a logger creation event to all registered listeners
702724
/// </summary>

‎src/log4net/Repository/Hierarchy/LoggerKey.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ namespace log4net.Repository.Hierarchy
4040
/// </remarks>
4141
/// <author>Nicko Cadell</author>
4242
/// <author>Gert Driesen</author>
43-
[DebuggerDisplay("{m_name}")]
43+
[DebuggerDisplay("{Name}")]
4444
internal readonly struct LoggerKey
4545
{
4646
/// <summary>
@@ -65,7 +65,7 @@ internal readonly struct LoggerKey
6565
/// <param name="name">The name of the logger.</param>
6666
internal LoggerKey(string name)
6767
{
68-
m_name = string.Intern(name);
68+
Name = string.Intern(name);
6969
m_hashCache = name.GetHashCode();
7070
}
7171

@@ -83,7 +83,11 @@ public override int GetHashCode()
8383
return m_hashCache;
8484
}
8585

86-
private readonly string m_name;
86+
/// <summary>
87+
/// Name of the Logger
88+
/// </summary>
89+
internal string Name { get; }
90+
8791
private readonly int m_hashCache;
8892

8993
public static Comparer ComparerInstance { get; } = new();
@@ -92,7 +96,7 @@ public sealed class Comparer : IEqualityComparer<LoggerKey>
9296
{
9397
public bool Equals(LoggerKey x, LoggerKey y)
9498
{
95-
return x.m_hashCache == y.m_hashCache && x.m_name == y.m_name;
99+
return x.m_hashCache == y.m_hashCache && x.Name == y.Name;
96100
}
97101

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

‎src/site/xdoc/release/release-notes.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ limitations under the License.
169169
Apache log4net 3.0.0 addresses reported issues:
170170
<section id="a3.0.0-bug" name="Bug fixes">
171171
<ul>
172+
<li>
173+
<a href="https://github.com/apache/logging-log4net/issues/156">Regression: Creating nested loggers in reverse order fails in 3.0.0-preview.1</a> (by @freeandnil)
174+
</li>
172175
</ul>
173176
</section>
174177
<section id="a3.0.0-enhancements" name="Enhancements">

0 commit comments

Comments
 (0)