-
-
Notifications
You must be signed in to change notification settings - Fork 337
Fix/reduce event loss on reconfigurationreduce silent log event loss during XmlConfigurator reconfiguration #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -594,4 +594,37 @@ | |
|
|
||
| CallAppenders(logEvent); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Atomically replaces all appenders with the provided collection. | ||
| /// </summary> | ||
| /// <param name="appenders">The new set of appenders to attach.</param> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// This method removes the existing appenders and attaches all new | ||
| /// appenders inside a single writer lock, minimizing the window | ||
| /// during which the logger has no appenders. This reduces silent log | ||
| /// event loss during reconfiguration. | ||
| /// </para> | ||
| /// </remarks> | ||
| public virtual void ReplaceAppenders(IEnumerable<IAppender> appenders) | ||
|
Check failure on line 610 in src/log4net/Repository/Hierarchy/Logger.cs
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please adjust the indentation |
||
| { | ||
| _appenderLock.AcquireWriterLock(); | ||
| try | ||
| { | ||
| _appenderAttachedImpl?.RemoveAllAppenders(); | ||
| _appenderAttachedImpl = null; | ||
|
|
||
| foreach (IAppender appender in appenders) | ||
| { | ||
| _appenderAttachedImpl ??= new(); | ||
| _appenderAttachedImpl.AddAppender(appender); | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| _appenderLock.ReleaseWriterLock(); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -389,43 +389,58 @@ protected void ParseRoot(XmlElement rootElement) | |||||
| /// <para> | ||||||
| /// Parse the child elements of a <logger> element. | ||||||
| /// </para> | ||||||
| /// <para> | ||||||
| /// Appenders are collected from XML first, then applied atomically via | ||||||
| /// <see cref="Logger.ReplaceAppenders"/>, minimizing the window during | ||||||
| /// which the logger has no appenders and silent log event loss can occur. | ||||||
| /// </para> | ||||||
| /// </remarks> | ||||||
| protected void ParseChildrenOfLoggerElement(XmlElement catElement, Logger log, bool isRoot) | ||||||
| { | ||||||
| // Remove all existing appenders from log. They will be | ||||||
| // reconstructed if need be. | ||||||
| log.EnsureNotNull().RemoveAllAppenders(); | ||||||
| log.EnsureNotNull(); | ||||||
| catElement.EnsureNotNull(); | ||||||
|
|
||||||
| // Phase 1: resolve all new appenders from XML *before* touching the | ||||||
| // live logger. This avoids the window where the logger has no appenders. | ||||||
| var newAppenders = new List<IAppender>(); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| foreach (XmlNode currentNode in catElement.EnsureNotNull().ChildNodes) | ||||||
| foreach (XmlNode currentNode in catElement.ChildNodes) | ||||||
| { | ||||||
| if (currentNode.NodeType == XmlNodeType.Element) | ||||||
| if (currentNode.NodeType != XmlNodeType.Element) | ||||||
| { | ||||||
| XmlElement currentElement = (XmlElement)currentNode; | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| if (currentElement.LocalName == AppenderRefTag) | ||||||
| { | ||||||
| string refName = currentElement.GetAttribute(RefAttr); | ||||||
| if (FindAppenderByReference(currentElement) is IAppender appender) | ||||||
| { | ||||||
| LogLog.Debug(_declaringType, $"Adding appender named [{refName}] to logger [{log.Name}]."); | ||||||
| log.AddAppender(appender); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| LogLog.Error(_declaringType, $"Appender named [{refName}] not found."); | ||||||
| } | ||||||
| } | ||||||
| else if (currentElement.LocalName is LevelTag or PriorityTag) | ||||||
| XmlElement currentElement = (XmlElement)currentNode; | ||||||
|
|
||||||
| if (currentElement.LocalName == AppenderRefTag) | ||||||
| { | ||||||
| string refName = currentElement.GetAttribute(RefAttr); | ||||||
| if (FindAppenderByReference(currentElement) is IAppender appender) | ||||||
| { | ||||||
| ParseLevel(currentElement, log, isRoot); | ||||||
| LogLog.Debug(_declaringType, $"Resolved appender [{refName}] for logger [{log.Name}]."); | ||||||
| newAppenders.Add(appender); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| SetParameter(currentElement, log); | ||||||
| LogLog.Error(_declaringType, $"Appender named [{refName}] not found."); | ||||||
| } | ||||||
| } | ||||||
| else if (currentElement.LocalName is LevelTag or PriorityTag) | ||||||
| { | ||||||
| ParseLevel(currentElement, log, isRoot); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| SetParameter(currentElement, log); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Phase 2: atomic swap — replace all appenders in one writer lock so | ||||||
| // the logger is never in a zero-appender state for longer than it takes | ||||||
| // to acquire and release the lock (microseconds, not milliseconds). | ||||||
| log.ReplaceAppenders(newAppenders); | ||||||
|
|
||||||
| if (log is IOptionHandler optionHandler) | ||||||
| { | ||||||
| optionHandler.ActivateOptions(); | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move to
logging-log4net/src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs
Line 151 in 103fd9d