Copilot commented on code in PR #322:
URL: https://github.com/apache/logging-log4net/pull/322#discussion_r4071440581
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -205,9 +208,24 @@ public void Configure(XmlElement? element)
}
}
+ ActivatePendingAppenders();
+
// Done reading config
}
+ /// <summary>
+ /// Activates the appenders parsed in this pass, in creation order.
+ /// </summary>
+ private void ActivatePendingAppenders()
+ {
+ _deferActivation = false;
+ foreach (IOptionHandler optionHandler in _pendingActivations)
+ {
+ optionHandler.ActivateOptions();
+ }
Review Comment:
`ParseAppender` previously called `ActivateOptions()` inside its
`try`/`catch (Exception e) when (!e.IsFatal())` (lines 299-359), so a non-fatal
activation failure such as the documented missing `UdpAppender.RemoteAddress`
was logged and that appender was not returned. This loop now runs outside that
error boundary, so the exception escapes `Configure` after `ReplaceAppenders`
has already attached the unactivated appender, leaving a partial configuration
(and potentially terminating a watch callback). Preserve the prior per-appender
failure handling and remove/close any appender whose deferred activation fails.
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -442,9 +466,7 @@ protected void ParseChildrenOfLoggerElement(XmlElement
catElement, Logger log, b
}
}
- // 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).
+ // Phase 2: swap in one writer lock, closing the outgoing appenders.
log.ReplaceAppenders(newAppenders);
Review Comment:
This publishes each pending appender to a live logger before
`ActivatePendingAppenders()` runs for the rest of the configuration. A
concurrent log call can therefore reach an unactivated `FileAppender`; its
`PreAppendCheck` lazily calls `PrepareWriter()`/`SafeOpenFile()`, so it can
reopen the file while another logger's old appender still holds the handle and
recreate the lock error this change is meant to prevent. Keep new appenders
unpublished until all outgoing appenders have been closed, or otherwise block
appends across the whole swap-and-activation phase.
--
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]
