FreeAndNil commented on code in PR #287:
URL: https://github.com/apache/logging-log4net/pull/287#discussion_r3053957896
##########
src/log4net/Repository/Hierarchy/Logger.cs:
##########
@@ -594,4 +594,37 @@ protected virtual void ForcedLog(LoggingEvent logEvent)
CallAppenders(logEvent);
}
-}
\ No newline at end of file
+
+ /// <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)
Review Comment:
please adjust the indentation
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -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>();
Review Comment:
```suggestion
List<IAppender> newAppenders = new();
```
##########
src/log4net/Repository/Hierarchy/Hierarchy.cs:
##########
@@ -283,7 +283,11 @@ public override void ResetConfiguration()
{
Root.Level = LevelMap.LookupWithDefault(Level.Debug);
Threshold = LevelMap.LookupWithDefault(Level.All);
-
+ // Reset the warning flag so diagnostics can fire during the next
+ // reconfiguration cycle. Without this, the "no appender" warning is
+ // silenced permanently after the first configuration.
+ EmittedNoAppenderWarning = false;
Review Comment:
Please move to
https://github.com/apache/logging-log4net/blob/103fd9da4b9aee5f0fb6853a2d055c66863e68fc/src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs#L151
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
