You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by fr...@apache.org on 2024/10/18 07:53:21 UTC
(logging-log4net) branch master updated: fixed IndexOutOfRangeException when creating child loggers multithreaded - fixes #197 (#198)
This is an automated email from the ASF dual-hosted git repository.
freeandnil pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4net.git
The following commit(s) were added to refs/heads/master by this push:
new 36ea5b45 fixed IndexOutOfRangeException when creating child loggers multithreaded - fixes #197 (#198)
36ea5b45 is described below
commit 36ea5b45c89ba2294fe3dbc96dcaf72366801790
Author: Jan Friedrich <43...@users.noreply.github.com>
AuthorDate: Fri Oct 18 09:53:17 2024 +0200
fixed IndexOutOfRangeException when creating child loggers multithreaded - fixes #197 (#198)
---
.editorconfig | 8 +-
src/log4net.Tests/Hierarchy/HierarchyTest.cs | 146 ++++++++++++---------
src/log4net/Repository/Hierarchy/Hierarchy.cs | 34 +++--
src/log4net/Repository/Hierarchy/ProvisionNode.cs | 37 +++++-
src/log4net/Repository/LoggerRepositorySkeleton.cs | 39 +++---
src/site/xdoc/release/release-notes.xml | 5 +-
6 files changed, 166 insertions(+), 103 deletions(-)
diff --git a/.editorconfig b/.editorconfig
index 06499e12..ceeea3be 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -158,5 +158,11 @@ dotnet_diagnostic.SYSLIB0050.severity = none
# SYSLIB0051: Type or member is obsolete
dotnet_diagnostic.SYSLIB0051.severity = none
+# KR1010: Use correct class field name prefixes
+dotnet_diagnostic.KR1010.severity = none
# KR1012: Classes should be sealed.
-dotnet_diagnostic.KR1012.severity = none
\ No newline at end of file
+dotnet_diagnostic.KR1012.severity = none
+# KR1013: Name base classes correctly
+dotnet_diagnostic.KR1013.severity = none
+# KR1037: Use EnvironmentHelper instead of DateTime.
+dotnet_diagnostic.KR1037.severity = none
\ No newline at end of file
diff --git a/src/log4net.Tests/Hierarchy/HierarchyTest.cs b/src/log4net.Tests/Hierarchy/HierarchyTest.cs
index 416a2f55..a2dbf285 100644
--- a/src/log4net.Tests/Hierarchy/HierarchyTest.cs
+++ b/src/log4net.Tests/Hierarchy/HierarchyTest.cs
@@ -20,6 +20,7 @@
*/
using System;
+using System.Threading.Tasks;
using System.Xml;
using log4net.Config;
using log4net.Repository;
@@ -29,7 +30,7 @@ using NUnit.Framework;
namespace log4net.Tests.Hierarchy;
[TestFixture]
-public class Hierarchy
+public class HierarchyTest
{
[Test]
public void SetRepositoryPropertiesInConfigFile()
@@ -37,19 +38,19 @@ public class Hierarchy
// LOG4NET-53: Allow repository properties to be set in the config file
XmlDocument log4NetConfig = new();
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"]!);
@@ -99,18 +100,18 @@ public class Hierarchy
{
XmlDocument log4NetConfig = new();
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"]!);
@@ -121,18 +122,18 @@ public class Hierarchy
{
XmlDocument log4NetConfig = new();
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"]!);
@@ -143,18 +144,18 @@ public class Hierarchy
{
XmlDocument log4NetConfig = new();
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"]!);
@@ -169,19 +170,44 @@ public class Hierarchy
{
XmlDocument log4NetConfig = new();
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>");
+ <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);
}
-}
+
+ /// <summary>
+ /// https://github.com/apache/logging-log4net/issues/197
+ /// IndexOutOfRangeException when creating child loggers multithreaded
+ /// </summary>
+ [Test]
+ public void CreateChildLoggersMultiThreaded()
+ {
+ XmlDocument log4NetConfig = new();
+ 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"]!);
+
+ Parallel.For(0, 100, i => Assert.AreEqual($"A.{i}", rep.GetLogger($"A.{i}").Name));
+ }
+}
\ No newline at end of file
diff --git a/src/log4net/Repository/Hierarchy/Hierarchy.cs b/src/log4net/Repository/Hierarchy/Hierarchy.cs
index 482d2cc7..fc3b4a43 100644
--- a/src/log4net/Repository/Hierarchy/Hierarchy.cs
+++ b/src/log4net/Repository/Hierarchy/Hierarchy.cs
@@ -41,22 +41,16 @@ public delegate void LoggerCreationEventHandler(object sender, LoggerCreationEve
/// </summary>
/// <remarks>
/// <para>
-/// A <see cref="Hierarchy.LoggerCreatedEvent"/> event is raised every time a
-/// <see cref="Logger"/> is created.
+/// A <see cref="Hierarchy.LoggerCreatedEvent"/> event is raised every time a <see cref="Logger"/> is created.
/// </para>
/// </remarks>
-public class LoggerCreationEventArgs : EventArgs
+/// <param name="log">The <see cref="Logger"/> that has been created.</param>
+public class LoggerCreationEventArgs(Logger log) : EventArgs
{
- /// <summary>
- /// Constructor
- /// </summary>
- /// <param name="log">The <see cref="Logger"/> that has been created.</param>
- public LoggerCreationEventArgs(Logger log) => Logger = log;
-
/// <summary>
/// Gets the <see cref="Logger"/> that has been created.
/// </summary>
- public Logger Logger { get; }
+ public Logger Logger { get; } = log;
}
/// <summary>
@@ -64,8 +58,7 @@ public class LoggerCreationEventArgs : EventArgs
/// </summary>
/// <remarks>
/// <para>
-/// <i>The casual user should not have to deal with this class
-/// directly.</i>
+/// <i>The casual user should not have to deal with this class directly.</i>
/// </para>
/// <para>
/// This class is specialized in retrieving loggers by name and also maintaining the logger
@@ -343,9 +336,10 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
/// The list returned is unordered but does not contain duplicates.
/// </para>
/// </remarks>
+ [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0305:Simplify collection initialization")]
public override IAppender[] GetAppenders()
{
- var appenderList = new HashSet<IAppender>();
+ HashSet<IAppender> appenderList = [];
CollectAppenders(appenderList, Root);
@@ -407,7 +401,7 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
/// </remarks>
protected void BasicRepositoryConfigure(params IAppender[] appenders)
{
- var configurationMessages = new List<LogLog>();
+ List<LogLog> configurationMessages = [];
using (new LogLog.LogReceivedAdapter(configurationMessages))
{
@@ -444,7 +438,7 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
/// </remarks>
protected void XmlRepositoryConfigure(System.Xml.XmlElement element)
{
- var configurationMessages = new List<LogLog>();
+ List<LogLog> configurationMessages = [];
using (new LogLog.LogReceivedAdapter(configurationMessages))
{
@@ -516,7 +510,7 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
name.EnsureNotNull();
factory.EnsureNotNull();
- var key = new LoggerKey(name);
+ LoggerKey key = new(name);
const int maxRetries = 5;
for (int i = 0; i < maxRetries; i++)
@@ -631,7 +625,7 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
{
string substr = name.Substring(0, i);
- var key = new LoggerKey(substr);
+ LoggerKey key = new(substr);
_loggers.TryGetValue(key, out object? node);
// Create a provision node for a future parent.
@@ -693,9 +687,11 @@ public class Hierarchy : LoggerRepositorySkeleton, IBasicRepositoryConfigurator,
/// c's parent field to log.
/// </para>
/// </remarks>
- private static void UpdateChildren(ProvisionNode pn, Logger log)
+ private static void UpdateChildren(ProvisionNode provisionNode, Logger log)
{
- foreach (Logger childLogger in pn)
+ provisionNode.ForEach(Update, log);
+
+ static void Update(Logger childLogger, Logger log)
{
// Unless this child already points to a correct (lower) parent,
// make log.Parent point to childLogger.Parent and childLogger.Parent to log.
diff --git a/src/log4net/Repository/Hierarchy/ProvisionNode.cs b/src/log4net/Repository/Hierarchy/ProvisionNode.cs
index 0858db37..3411775f 100644
--- a/src/log4net/Repository/Hierarchy/ProvisionNode.cs
+++ b/src/log4net/Repository/Hierarchy/ProvisionNode.cs
@@ -17,6 +17,8 @@
//
#endregion
+using System;
+using System.Collections;
using System.Collections.Generic;
namespace log4net.Repository.Hierarchy;
@@ -36,14 +38,41 @@ namespace log4net.Repository.Hierarchy;
/// </remarks>
/// <author>Nicko Cadell</author>
/// <author>Gert Driesen</author>
-internal sealed class ProvisionNode : List<Logger>
+internal sealed class ProvisionNode
{
+ private readonly List<Logger> _loggers;
+
/// <summary>
/// Create a new provision node with child node
/// </summary>
/// <param name="log">A child logger to add to this node.</param>
- internal ProvisionNode(Logger log)
+ internal ProvisionNode(Logger log) => _loggers = [log];
+
+ /// <summary>
+ /// Add a <see cref="Logger"/> to the internal List
+ /// </summary>
+ /// <param name="log">Logger</param>
+ internal void Add(Logger log)
+ {
+ lock (((IList)_loggers).SyncRoot)
+ {
+ _loggers.Add(log);
+ }
+ }
+
+ /// <summary>
+ /// Calls <paramref name="callback"/> for each logger in the internal list
+ /// </summary>
+ /// <param name="callback">Callback to execute</param>
+ /// <param name="parent">Parant logger</param>
+ internal void ForEach(Action<Logger, Logger> callback, Logger parent)
{
- Add(log);
+ lock (((IList)_loggers).SyncRoot)
+ {
+ foreach (Logger log in _loggers)
+ {
+ callback(log, parent);
+ }
+ }
}
-}
+}
\ No newline at end of file
diff --git a/src/log4net/Repository/LoggerRepositorySkeleton.cs b/src/log4net/Repository/LoggerRepositorySkeleton.cs
index b67befc0..37d15968 100644
--- a/src/log4net/Repository/LoggerRepositorySkeleton.cs
+++ b/src/log4net/Repository/LoggerRepositorySkeleton.cs
@@ -447,10 +447,7 @@ public abstract class LoggerRepositorySkeleton : ILoggerRepository, IFlushable
/// </para>
/// </remarks>
protected virtual void OnConfigurationReset(EventArgs? e)
- {
- e ??= EventArgs.Empty;
- ConfigurationReset?.Invoke(this, e);
- }
+ => ConfigurationReset?.Invoke(this, e ?? EventArgs.Empty);
/// <summary>
/// Notify the registered listeners that the repository has had its configuration changed
@@ -462,10 +459,7 @@ public abstract class LoggerRepositorySkeleton : ILoggerRepository, IFlushable
/// </para>
/// </remarks>
protected virtual void OnConfigurationChanged(EventArgs? e)
- {
- e ??= EventArgs.Empty;
- ConfigurationChanged?.Invoke(this, e);
- }
+ => ConfigurationChanged?.Invoke(this, e ?? EventArgs.Empty);
/// <summary>
/// Raise a configuration changed event on this repository
@@ -477,20 +471,22 @@ public abstract class LoggerRepositorySkeleton : ILoggerRepository, IFlushable
/// raise this event notification to notify listeners.
/// </para>
/// </remarks>
- public void RaiseConfigurationChanged(EventArgs e)
- {
- OnConfigurationChanged(e);
- }
+ public void RaiseConfigurationChanged(EventArgs e) => OnConfigurationChanged(e);
private static int GetWaitTime(DateTime startTimeUtc, int millisecondsTimeout)
{
- if (millisecondsTimeout == Timeout.Infinite) return Timeout.Infinite;
- if (millisecondsTimeout == 0) return 0;
+ if (millisecondsTimeout == Timeout.Infinite)
+ {
+ return Timeout.Infinite;
+ }
+ if (millisecondsTimeout == 0)
+ {
+ return 0;
+ }
int elapsedMilliseconds = (int)(DateTime.UtcNow - startTimeUtc).TotalMilliseconds;
int timeout = millisecondsTimeout - elapsedMilliseconds;
- if (timeout < 0) timeout = 0;
- return timeout;
+ return (timeout < 0) ? 0 : timeout;
}
/// <summary>
@@ -501,7 +497,11 @@ public abstract class LoggerRepositorySkeleton : ILoggerRepository, IFlushable
/// <returns><c>True</c> if all logging events were flushed successfully, else <c>false</c>.</returns>
public bool Flush(int millisecondsTimeout)
{
- if (millisecondsTimeout < -1) throw new ArgumentOutOfRangeException(nameof(millisecondsTimeout), "Timeout must be -1 (Timeout.Infinite) or non-negative");
+ if (millisecondsTimeout < -1)
+ {
+ throw new ArgumentOutOfRangeException(nameof(millisecondsTimeout),
+ "Timeout must be -1 (Timeout.Infinite) or non-negative");
+ }
// Assume success until one of the appenders fails
bool result = true;
@@ -520,7 +520,10 @@ public abstract class LoggerRepositorySkeleton : ILoggerRepository, IFlushable
if (appender is BufferingAppenderSkeleton)
{
int timeout = GetWaitTime(startTimeUtc, millisecondsTimeout);
- if (!flushable.Flush(timeout)) result = false;
+ if (!flushable.Flush(timeout))
+ {
+ result = false;
+ }
}
}
diff --git a/src/site/xdoc/release/release-notes.xml b/src/site/xdoc/release/release-notes.xml
index 4e4690a5..65afdefc 100644
--- a/src/site/xdoc/release/release-notes.xml
+++ b/src/site/xdoc/release/release-notes.xml
@@ -29,7 +29,10 @@ limitations under the License.
<section id="a3.0.2-bug" name="Bug fixes">
<ul>
<li>
- <a href="https://github.com/apache/logging-log4net/issues/tbd">tbd</a> (by tbd)
+ <a href="https://github.com/apache/logging-log4net/issues/194">ObjectDisposedException in TelnetAppender (3.0.1)</a> (reported by VianneyTremelo, fixed by @FreeAndNil in https://github.com/apache/logging-log4net/pull/195)
+ </li>
+ <li>
+ <a href="https://github.com/apache/logging-log4net/issues/197">IndexOutOfRangeException when creating child loggers multithreaded</a> (reported by @FreeAndNil, fixed by @FreeAndNil in https://github.com/apache/logging-log4net/pull/198)
</li>
</ul>
</section>