HangfireIO / Cronos

A fully-featured .NET library for working with Cron expressions. Built with time zones in mind and intuitively handles daylight saving time transitions
MIT License
1.01k stars 118 forks source link

Fix a bug related to rare DST cases because of wrong DateTime convertation between time zones on Windows #37

Closed aidmsu closed 3 years ago

aidmsu commented 3 years ago

This PR fixes rare case reproduced on Windows and related to daylight saving time (DST) transiton.

DateTime convertation bug on Windows

There are 49 of 141 Windows time zones which have not totally accurate daylight transtion time information.

Consider time zone Chile Time (Windows time zone id: Pacific SA Standard Time) as example. Chile Standard Time (CST) is 4 hours before UTC, and Chile Summer Time (CLST) - 3 hours before UTC.

1. DST start case for Chile time zone.

In 2021 DST starts when clocks jump forward from 2021-09-05 00:00 -4:00 CST to 2021-09-05 01:00 -3:00 CLST. So local time [2021-09-05 00:00, 2021-09-05 01:00) does not exist.

time -> _____23:59:59.9999999 -04:00 CST////invalid////01:00:00.0000000 -03:00 CLST

But in Windows time zone data base transition time is not 00:00:00 but 23:59:59.9990000 - 10000 ticks before. As as result, convertation from 2021-09-05 03:59:59.9990000 UTC to Chile time equals 2021-09-05 00:59:59.9990000 -3:00 CLST instead of 2021-09-04 23:59:59.9990000 -4:00 CST. See code below:

var chileTimeZone = TimeZoneInfo.FindSystemTimeZoneById("Pacific SA Standard Time");

var utcTime = DateTime.Parse("2021-09-05 03:59:59.9990000"); // 23:59:59.999000 local time
var result = TimeZoneInfo.ConvertTimeFromUtc(utcTime, chileTimeZone);
// result equals 2021-09-05 00:59:59.9990000 -3:00 CLST instead of 2021-09-05 23:59:59.9990000 -4:00 CST

2. DST end case for Chile time zone.

It's the similar behavior for DST end. Clocks jump backward from 2021-04-04 00:00 -3:00 CLST to 2021-04-03 23:00 -4:00 CLT. So local time [2021-04-03 23:00, 2021-04-04 00:00) is ambiguous.

time -> 23:00 -03:0023:59:59.9999999 -03:00 CST -> 23:00:00.0000000 -04:00 CLST

And convertation from 2021-04-04 02:59:59.9990000 UTC to Chile time equals 2021-04-04 22:59:59.9990000 -4:00 CLT instead of 2021-04-04 23:59:59.9990000 -3:00 CST.

Consequences for Cronos

Due to these rounding errors Cronos can skip some occurrences when DST starts:

var chileTimeZone = TimeZoneInfo.FindSystemTimeZoneById("Pacific SA Standard Time");
var cronExpression = CronExpression.Parse("30 0 * * *"); // every day at 0:30 am

// 1 tick before DST start, local time 23:59:59.9999999
var fromUtc = DateTimeOffset.Parse("2021-09-05 03:59:59.9999999Z"); 

var next = cronExpression.GetNextOccurrence(fromUtc, chileTimeZone);
// next = "2021-09-06 00:30" instead of "2021-09-05 01:00"
// Occurence skipped on 2021-09-05.

Or even get occurrence from the past when DST ends:

var chileTimeZone = TimeZoneInfo.FindSystemTimeZoneById("Pacific SA Standard Time");
var cronExpression = CronExpression.Parse("0 23 * * *"); // every day at 23:00 am

// 1 tick before DST end, local time 2021-04-03 23:59:59.9999999 -03:00
var fromUtc = DateTimeOffset.Parse("2021-04-04 02:59:59.9999999Z"); 

var next = cronExpression.GetNextOccurrence(fromUtc, chileTimeZone);
// next = "2021-04-03 23:00 -03:00" instead of "2021-04-04 23:00 -04:00"
// Occurence 2021-04-03 23:00:00.0000000 -03:00 is earlier than fromUtc: 2021-04-03 23:59:59.9999999 -03:00.

Decision

We can just floor input time to seconds and avoid such behavior.

In case of DST start we floor 2021-09-05 03:59:59.9990000 UTC to 2021-09-05 03:59:59.0000000 UTC. Then convertation from 2021-09-05 03:59:59.0000000 UTC to Chile time equals 2021-09-04 23:59:59.0000000 -4:00 CST instead of wrong 2021-09-05 00:59:59.9990000 -3:00 CLST.

Also we have to set inclusive = false. Otherwise floored time can match cron expression and be returned as result, but it'l be wrong because result will be less than passed from argument.

odinserj commented 3 years ago

Awesome, thanks a lot for the investigation! I have some questions, since it potentially can break some use cases, so we need to think and implement it carefully.

There are 49 of 141 Windows time zones which have not totally accurate daylight transition time information.

Is this information available anywhere to read about it, e.g. how did you learn about these numbers?

But in Windows time zone data base transition time is not 00:00:00 but 23:59:59.9990000 - 10000 ticks before.

Does TimeZoneInfo provide any API to learn that a particular time zone has DST that starts from 00:00:00 or 23:59:59.999?

We can just floor input time to seconds and avoid such behaviour.

Does this decision cause any breaking changes? Consider we are using the following code (I understand it can cause problems with inclusive: true, but anyway):

var fromUtc = /* Some Date */;
var expression = /* Some Expression */;

while (/* SomeCondition */)
{
    fromUtc = expression.GetNextOccurrence(fromUtc.AddTicks(1), chileTimeZone, inclusive: true);
}

As far as I understand these changes will break the behaviour (more over, it will cause an endless loop), right?

aidmsu commented 3 years ago

Is this information available anywhere to read about it, e.g. how did you learn about these numbers?

Does TimeZoneInfo provide any API to learn that a particular time zone has DST that starts from 00:00:00 or 23:59:59.999?

I did not find any official information about it. But TimeZoneInfo provides API to get DST time transisition for certain date. And I used this API to get a list of time zones with wrong info about DST. Firstly I added to the list a few extra time zones by mistake. Now, I rewrote my code and got 17 such zones (Last time I took into account zones with rule.DaylightDelta == TimeSpan.Zero and got 49 time zones instead of 17):

var allZones = TimeZoneInfo.GetSystemTimeZones();
var wrongDstInfoZones = zones.Where(HasWrongDstInfo).ToList();

Console.WriteLine($"There are {wrongDstInfoZones.Count} of {zones.Count} time zones with wrong DST info.");
// There are 17 of 141 time zones with wrong DST info.

...

public static bool HasWrongDstInfo(TimeZoneInfo zone)
{
    var rules = zone.GetAdjustmentRules();

    return zone.SupportsDaylightSavingTime && rules.Any(IsAdjustmentRuleWrong));
}

public static bool IsAdjustmentRuleWrong(TimeZoneInfo.AdjustmentRule rule)
{
    // If there was not DST in the period we don't have to check transition time.
    if (rule.DaylightDelta == TimeSpan.Zero) return false;

    return IsTransitionTimeWrong(rule.DaylightTransitionStart) || IsTransitionTimeWrong(rule.DaylightTransitionEnd);
}

public static bool IsTransitionTimeWrong(TimeZoneInfo.TransitionTime timeOfDay)
{
    var oneSecond = TimeSpan.FromSeconds(1);

    var timeOfDay = transitionTime.TimeOfDay;
    return timeOfDay.Ticks % oneSecond.Ticks != 0;
}

And there are only 6 time zones that have wrong actual (that applies now, not in the past) rule with wrong DST info (I published the code to GitHub and .NET Fiddle if somebody wants to check it).

  1. Paraguay Standard Time. Adjustment rule for (2021-01-01, 9999-12-31) period: Transition start: the First Saturday of October at 23:59:59.9990000 Transition end: the Last Saturday of March at 23:59:59.9990000.

  2. Pacific SA Standard Time. Adjustment rule for (2019-01-01, 9999-12-31) period. Transition start: the First Saturday of September at 23:59:59.9990000 Transition end: the First Saturday of April at 23:59:59.9990000.

  3. Jordan Standard Time. Adjustment rule for (2014-01-01, 9999-12-31) period. Transition start: the Last Thursday of March at 23:59:59.9990000 Transition end: the Last Friday of October at 01:00:00.0000000.

  4. Middle East Standard Time. Adjustment rule for (2021-01-01, 9999-12-31) period. Transition start: the Last Saturday of March at 23:59:59.9990000 Transition end: the Last Saturday of October at 23:59:59.9990000.

  5. Syria Standard Time. Adjustment rule for (2020-01-01, 9999-12-31) period. Transition start: the Last Friday of March at 00:00:00.0000000 Transition end: the Last Thursday of October at 23:59:59.9990000.

  6. Iran Standard Time. Adjustment rule for (2024-01-01, 9999-12-31) period. Transition start: the Third Thursday of March at 00:00:00.0000000 Transition end: the Third Friday of September at 23:59:59.9990000.

It's interesting that all incorrect DST time transitions are exactly 23:59:59.9990000 - 10000 ticks before midnight.

Does this decision cause any breaking changes? Consider we are using the following code (I understand it can cause problems with inclusive: true, but anyway):

The changes should not lead to breaking changes. In your example the behavior will be as the same as before the changes. You add 1 tick to avoid endless loop and get new result at each iteration with inclusive = true. After changes it will be achieved by setting inclusive = false inside GetNextOccurrence method.

So the following code:

var fromUtc = /* Some Date */;
var expression = /* Some Expression */;

while (/* SomeCondition */)
{
    fromUtc = expression.GetNextOccurrence(fromUtc.AddTicks(1), chileTimeZone, inclusive: true);
}

is equivalent (almost, actually we don't round fromUtc if it doesn't have extra ticks) to:

var fromUtc = /* Some Date */;
var expression = /* Some Expression */;

while (/* SomeCondition */)
{
    var newFromUtc = fromUtc.AddTicks(1);
    newFromUtc = RoundToSeconds(newFromUtc);

    // Execute with inclusive = false
    fromUtc = expression.GetNextOccurrence(newFromUtc, chileTimeZone, inclusive: false);
}

I added tests to check making progress inside iteration.