Skip to content

Commit 36ea5b4

Browse files
FreeAndNilFreeAndNil
authored
fixed IndexOutOfRangeException when creating child loggers multithreaded - fixes #197 (#198)
1 parent ab16a6e commit 36ea5b4

File tree

6 files changed

+166
-103
lines changed

6 files changed

+166
-103
lines changed

‎.editorconfig

+7-1
Original file line numberDiff line numberDiff line change
@@ -158,5 +158,11 @@ dotnet_diagnostic.SYSLIB0050.severity = none
158158
# SYSLIB0051: Type or member is obsolete
159159
dotnet_diagnostic.SYSLIB0051.severity = none
160160

161+
# KR1010: Use correct class field name prefixes
162+
dotnet_diagnostic.KR1010.severity = none
161163
# KR1012: Classes should be sealed.
162-
dotnet_diagnostic.KR1012.severity = none
164+
dotnet_diagnostic.KR1012.severity = none
165+
# KR1013: Name base classes correctly
166+
dotnet_diagnostic.KR1013.severity = none
167+
# KR1037: Use EnvironmentHelper instead of DateTime.
168+
dotnet_diagnostic.KR1037.severity = none

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

+86-60
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
*/
2121

2222
using System;
23+
using System.Threading.Tasks;
2324
using System.Xml;
2425
using log4net.Config;
2526
using log4net.Repository;
@@ -29,27 +30,27 @@
2930
namespace log4net.Tests.Hierarchy;
3031

3132
[TestFixture]
32-
public class Hierarchy
33+
public class HierarchyTest
3334
{
3435
[Test]
3536
public void SetRepositoryPropertiesInConfigFile()
3637
{
3738
// LOG4NET-53: Allow repository properties to be set in the config file
3839
XmlDocument log4NetConfig = new();
3940
log4NetConfig.LoadXml(@"
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>");
41+
<log4net>
42+
<property>
43+
<key value=""two-plus-two"" />
44+
<value value=""4"" />
45+
</property>
46+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
47+
<layout type=""log4net.Layout.SimpleLayout"" />
48+
</appender>
49+
<root>
50+
<level value=""ALL"" />
51+
<appender-ref ref=""StringAppender"" />
52+
</root>
53+
</log4net>");
5354

5455
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
5556
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
@@ -99,18 +100,18 @@ public void LoggerNameCanConsistOfASingleDot()
99100
{
100101
XmlDocument log4NetConfig = new();
101102
log4NetConfig.LoadXml(@"
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>");
103+
<log4net>
104+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
105+
<layout type=""log4net.Layout.SimpleLayout"" />
106+
</appender>
107+
<root>
108+
<level value=""ALL"" />
109+
<appender-ref ref=""StringAppender"" />
110+
</root>
111+
<logger name=""."">
112+
<level value=""WARN"" />
113+
</logger>
114+
</log4net>");
114115

115116
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
116117
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
@@ -121,18 +122,18 @@ public void LoggerNameCanConsistOfASingleNonDot()
121122
{
122123
XmlDocument log4NetConfig = new();
123124
log4NetConfig.LoadXml(@"
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>");
125+
<log4net>
126+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
127+
<layout type=""log4net.Layout.SimpleLayout"" />
128+
</appender>
129+
<root>
130+
<level value=""ALL"" />
131+
<appender-ref ref=""StringAppender"" />
132+
</root>
133+
<logger name=""L"">
134+
<level value=""WARN"" />
135+
</logger>
136+
</log4net>");
136137

137138
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
138139
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
@@ -143,18 +144,18 @@ public void LoggerNameCanContainSequenceOfDots()
143144
{
144145
XmlDocument log4NetConfig = new();
145146
log4NetConfig.LoadXml(@"
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>");
147+
<log4net>
148+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
149+
<layout type=""log4net.Layout.SimpleLayout"" />
150+
</appender>
151+
<root>
152+
<level value=""ALL"" />
153+
<appender-ref ref=""StringAppender"" />
154+
</root>
155+
<logger name=""L..M"">
156+
<level value=""WARN"" />
157+
</logger>
158+
</log4net>");
158159

159160
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
160161
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
@@ -169,19 +170,44 @@ public void CreateNestedLoggersInReverseOrder()
169170
{
170171
XmlDocument log4NetConfig = new();
171172
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>");
173+
<log4net>
174+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
175+
<layout type=""log4net.Layout.SimpleLayout"" />
176+
</appender>
177+
<root>
178+
<level value=""ALL"" />
179+
<appender-ref ref=""StringAppender"" />
180+
</root>
181+
</log4net>");
181182

182183
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
183184
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
184185
Assert.AreEqual("A.B.C", rep.GetLogger("A.B.C").Name);
185186
Assert.AreEqual("A.B", rep.GetLogger("A.B").Name);
186187
}
187-
}
188+
189+
/// <summary>
190+
/// https://github.com/apache/logging-log4net/issues/197
191+
/// IndexOutOfRangeException when creating child loggers multithreaded
192+
/// </summary>
193+
[Test]
194+
public void CreateChildLoggersMultiThreaded()
195+
{
196+
XmlDocument log4NetConfig = new();
197+
log4NetConfig.LoadXml(@"
198+
<log4net>
199+
<appender name=""StringAppender"" type=""log4net.Tests.Appender.StringAppender, log4net.Tests"">
200+
<layout type=""log4net.Layout.SimpleLayout"" />
201+
</appender>
202+
<root>
203+
<level value=""ALL"" />
204+
<appender-ref ref=""StringAppender"" />
205+
</root>
206+
</log4net>");
207+
208+
ILoggerRepository rep = LogManager.CreateRepository(Guid.NewGuid().ToString());
209+
XmlConfigurator.Configure(rep, log4NetConfig["log4net"]!);
210+
211+
Parallel.For(0, 100, i => Assert.AreEqual($"A.{i}", rep.GetLogger($"A.{i}").Name));
212+
}
213+
}

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

+15-19
Original file line numberDiff line numberDiff line change
@@ -41,31 +41,24 @@ namespace log4net.Repository.Hierarchy;
4141
/// </summary>
4242
/// <remarks>
4343
/// <para>
44-
/// A <see cref="Hierarchy.LoggerCreatedEvent"/> event is raised every time a
45-
/// <see cref="Logger"/> is created.
44+
/// A <see cref="Hierarchy.LoggerCreatedEvent"/> event is raised every time a <see cref="Logger"/> is created.
4645
/// </para>
4746
/// </remarks>
48-
public class LoggerCreationEventArgs : EventArgs
47+
/// <param name="log">The <see cref="Logger"/> that has been created.</param>
48+
public class LoggerCreationEventArgs(Logger log) : EventArgs
4949
{
50-
/// <summary>
51-
/// Constructor
52-
/// </summary>
53-
/// <param name="log">The <see cref="Logger"/> that has been created.</param>
54-
public LoggerCreationEventArgs(Logger log) => Logger = log;
55-
5650
/// <summary>
5751
/// Gets the <see cref="Logger"/> that has been created.
5852
/// </summary>
59-
public Logger Logger { get; }
53+
public Logger Logger { get; } = log;
6054
}
6155

6256
/// <summary>
6357
/// Hierarchical organization of loggers
6458
/// </summary>
6559
/// <remarks>
6660
/// <para>
67-
/// <i>The casual user should not have to deal with this class
68-
/// directly.</i>
61+
/// <i>The casual user should not have to deal with this class directly.</i>
6962
/// </para>
7063
/// <para>
7164
/// This class is specialized in retrieving loggers by name and also maintaining the logger
@@ -343,9 +336,10 @@ public override void Log(LoggingEvent logEvent)
343336
/// The list returned is unordered but does not contain duplicates.
344337
/// </para>
345338
/// </remarks>
339+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0305:Simplify collection initialization")]
346340
public override IAppender[] GetAppenders()
347341
{
348-
var appenderList = new HashSet<IAppender>();
342+
HashSet<IAppender> appenderList = [];
349343

350344
CollectAppenders(appenderList, Root);
351345

@@ -407,7 +401,7 @@ void IBasicRepositoryConfigurator.Configure(params IAppender[] appenders)
407401
/// </remarks>
408402
protected void BasicRepositoryConfigure(params IAppender[] appenders)
409403
{
410-
var configurationMessages = new List<LogLog>();
404+
List<LogLog> configurationMessages = [];
411405

412406
using (new LogLog.LogReceivedAdapter(configurationMessages))
413407
{
@@ -444,7 +438,7 @@ protected void BasicRepositoryConfigure(params IAppender[] appenders)
444438
/// </remarks>
445439
protected void XmlRepositoryConfigure(System.Xml.XmlElement element)
446440
{
447-
var configurationMessages = new List<LogLog>();
441+
List<LogLog> configurationMessages = [];
448442

449443
using (new LogLog.LogReceivedAdapter(configurationMessages))
450444
{
@@ -516,7 +510,7 @@ public Logger GetLogger(string name, ILoggerFactory factory)
516510
name.EnsureNotNull();
517511
factory.EnsureNotNull();
518512

519-
var key = new LoggerKey(name);
513+
LoggerKey key = new(name);
520514

521515
const int maxRetries = 5;
522516
for (int i = 0; i < maxRetries; i++)
@@ -631,7 +625,7 @@ private void UpdateParents(Logger log)
631625
{
632626
string substr = name.Substring(0, i);
633627

634-
var key = new LoggerKey(substr);
628+
LoggerKey key = new(substr);
635629
_loggers.TryGetValue(key, out object? node);
636630

637631
// Create a provision node for a future parent.
@@ -693,9 +687,11 @@ private void UpdateParents(Logger log)
693687
/// c's parent field to log.
694688
/// </para>
695689
/// </remarks>
696-
private static void UpdateChildren(ProvisionNode pn, Logger log)
690+
private static void UpdateChildren(ProvisionNode provisionNode, Logger log)
697691
{
698-
foreach (Logger childLogger in pn)
692+
provisionNode.ForEach(Update, log);
693+
694+
static void Update(Logger childLogger, Logger log)
699695
{
700696
// Unless this child already points to a correct (lower) parent,
701697
// make log.Parent point to childLogger.Parent and childLogger.Parent to log.

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

+33-4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
//
1818
#endregion
1919

20+
using System;
21+
using System.Collections;
2022
using System.Collections.Generic;
2123

2224
namespace log4net.Repository.Hierarchy;
@@ -36,14 +38,41 @@ namespace log4net.Repository.Hierarchy;
3638
/// </remarks>
3739
/// <author>Nicko Cadell</author>
3840
/// <author>Gert Driesen</author>
39-
internal sealed class ProvisionNode : List<Logger>
41+
internal sealed class ProvisionNode
4042
{
43+
private readonly List<Logger> _loggers;
44+
4145
/// <summary>
4246
/// Create a new provision node with child node
4347
/// </summary>
4448
/// <param name="log">A child logger to add to this node.</param>
45-
internal ProvisionNode(Logger log)
49+
internal ProvisionNode(Logger log) => _loggers = [log];
50+
51+
/// <summary>
52+
/// Add a <see cref="Logger"/> to the internal List
53+
/// </summary>
54+
/// <param name="log">Logger</param>
55+
internal void Add(Logger log)
56+
{
57+
lock (((IList)_loggers).SyncRoot)
58+
{
59+
_loggers.Add(log);
60+
}
61+
}
62+
63+
/// <summary>
64+
/// Calls <paramref name="callback"/> for each logger in the internal list
65+
/// </summary>
66+
/// <param name="callback">Callback to execute</param>
67+
/// <param name="parent">Parant logger</param>
68+
internal void ForEach(Action<Logger, Logger> callback, Logger parent)
4669
{
47-
Add(log);
70+
lock (((IList)_loggers).SyncRoot)
71+
{
72+
foreach (Logger log in _loggers)
73+
{
74+
callback(log, parent);
75+
}
76+
}
4877
}
49-
}
78+
}

0 commit comments

Comments
 (0)