FreeAndNil commented on code in PR #319:
URL: https://github.com/apache/logging-log4net/pull/319#discussion_r4065007596
##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -1114,7 +1157,10 @@ protected void RollOverTime(bool fileIsOpen)
RollFile(from, to);
}
- RollFile(File!, _scheduledFilename!);
+ if (!TryRollFile(File!, _scheduledFilename!))
+ {
+ RecordFailedBaseRename(File!, _scheduledFilename!,
wasBackupCountReverted: false);
+ }
Review Comment:
Right, and reproduced. `ExistingInit` returns as soon as a pending rename is
recorded, so the
startup path never rolls a second time. Test:
`AFailedStartupRollLeavesTheArchiveAlone`.
##########
src/log4net/Appender/RollingFileAppender.cs:
##########
@@ -587,6 +614,12 @@ protected override void OpenFile(string fileName, bool
append)
{
fileName = GetNextOutputFileName(fileName);
+ // A rename that failed left its file where it was. Never truncate that
one, whatever
+ // AppendToFile says, or the roll destroys what it could not move.
+ append = append
+ || (_pendingRename is not null
+ && string.Equals(fileName, _pendingRename.From,
StringComparison.Ordinal));
Review Comment:
Right, and my earlier reply on the startup fix was wrong: it said the
approach mutates no user
setting. It did, in `base.OpenFile` (`FileAppender.cs:1292`).
`RollingFileAppender.OpenFile` now restores the configured value in a
`finally`, since that call can
throw after assigning it. Every ordinary roll hit this too, not just a
failed one, so a configured
`AppendToFile = true` became false for good after the first roll. Tests:
`AFailedOpenLeavesAppendToFileAsConfigured` and one assertion in
`RollingCombinedWithPreserveExtension`.
--
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]
