Severity: Warning
File: src/Servy.Core/IO/RotatingStreamWriter.cs lines 80-103
Description:
The constructor builds the _timeProvider lambda that captures this._useLocalTimeForRotation before that field is actually assigned, then invokes the lambda one line later:
public RotatingStreamWriter(
string path,
bool enableSizeRotation,
long rotationSizeInBytes,
bool enableDateRotation,
DateRotationType dateRotationType,
int maxRotations,
bool useLocalTimeForRotation,
Func<DateTime>? timeProvider = null)
{
...
_dateRotationType = dateRotationType;
// Initialize the time provider. Defaults to the system clock based on configuration.
_timeProvider = timeProvider ?? (() => _useLocalTimeForRotation ? DateTime.Now : DateTime.UtcNow); // line 97
var now = _timeProvider(); // line 99 — lambda runs HERE
var lastWriteTime = useLocalTimeForRotation ? File.GetLastWriteTime(path) : File.GetLastWriteTimeUtc(path);
_lastRotationDate = File.Exists(path) ? lastWriteTime : now; // line 101 — 'now' used HERE
_maxRotations = maxRotations;
_useLocalTimeForRotation = useLocalTimeForRotation; // line 103 — field assigned HERE (too late)
}When the lambda at line 97 is invoked at line 99, _useLocalTimeForRotation still has its default value false, so the lambda returns DateTime.UtcNow regardless of the caller's useLocalTimeForRotation argument.
Concrete impact: when a caller passes useLocalTimeForRotation: true for a path that does not yet exist (File.Exists(path) == false), _lastRotationDate on line 101 is set to DateTime.UtcNow instead of DateTime.Now. The first date-rotation boundary is then off by the local-vs-UTC offset of the host (e.g. for a daily rotation in UTC+2, midnight local = 22:00 UTC, so the first rotation can occur up to 2 h earlier or later than expected, depending on when the service started).
Subsequent invocations of _timeProvider() after construction completes are fine — by then the field is correctly assigned.
The sibling parameter useLocalTimeForRotation is used correctly on line 100 to select GetLastWriteTime vs GetLastWriteTimeUtc, which shows the intent of the constructor.
Suggested fix:
Assign the field before constructing the lambda:
_dateRotationType = dateRotationType;
_useLocalTimeForRotation = useLocalTimeForRotation; // assign BEFORE the lambda captures 'this'
_timeProvider = timeProvider ?? (() => _useLocalTimeForRotation ? DateTime.Now : DateTime.UtcNow);
var now = _timeProvider();
var lastWriteTime = _useLocalTimeForRotation ? File.GetLastWriteTime(path) : File.GetLastWriteTimeUtc(path);
_lastRotationDate = File.Exists(path) ? lastWriteTime : now;
_maxRotations = maxRotations;
Capturing the parameter by value in the lambda (var localFlag = useLocalTimeForRotation; then () => localFlag ? ...) works too, but re-ordering the assignment is the minimal diff.
Severity: Warning
File:
src/Servy.Core/IO/RotatingStreamWriter.cslines 80-103Description:
The constructor builds the
_timeProviderlambda that capturesthis._useLocalTimeForRotationbefore that field is actually assigned, then invokes the lambda one line later:When the lambda at line 97 is invoked at line 99,
_useLocalTimeForRotationstill has its default valuefalse, so the lambda returnsDateTime.UtcNowregardless of the caller'suseLocalTimeForRotationargument.Concrete impact: when a caller passes
useLocalTimeForRotation: truefor a path that does not yet exist (File.Exists(path) == false),_lastRotationDateon line 101 is set toDateTime.UtcNowinstead ofDateTime.Now. The first date-rotation boundary is then off by the local-vs-UTC offset of the host (e.g. for a daily rotation in UTC+2, midnight local = 22:00 UTC, so the first rotation can occur up to 2 h earlier or later than expected, depending on when the service started).Subsequent invocations of
_timeProvider()after construction completes are fine — by then the field is correctly assigned.The sibling parameter
useLocalTimeForRotationis used correctly on line 100 to selectGetLastWriteTimevsGetLastWriteTimeUtc, which shows the intent of the constructor.Suggested fix:
Assign the field before constructing the lambda:
Capturing the parameter by value in the lambda (
var localFlag = useLocalTimeForRotation;then() => localFlag ? ...) works too, but re-ordering the assignment is the minimal diff.