Skip to content

Daylight Savings Time Fix#21

Open
wishdev wants to merge 2 commits intoexander77:masterfrom
wishdev:dst
Open

Daylight Savings Time Fix#21
wishdev wants to merge 2 commits intoexander77:masterfrom
wishdev:dst

Conversation

@wishdev
Copy link
Copy Markdown

@wishdev wishdev commented Jun 4, 2025

This addresses an issue with the crossing of daylight savings time boundaries.

Specifically the test on line 912 skips the entire day of 3/10/2019 without the fix.

As I believe I understand the issue is as follows (for the test on line 912)

The code creates a struct tm on 3/9 and then as it works to locate the next time (which should be 3/10 at 6AM) - it sets the struct to 23:00 on 3/10 (to change days) but it has the isdst flag as 0.
Then the system (i.e. not this library) reads that time and readjusts the struct to 0 hours on 3/11 with the isdst flag as 1.
The code then zeros out the hours and tries to start looking for the next match. But it's jumped over the 10th due to the daylight savings jump, obviously not the desired situation.

Switching the isdst flag to -1 when we set a field allows the system to properly switch into the 10th and therefore the struct gets to the correct spot and works properly.

@exander77
Copy link
Copy Markdown
Owner

Thank you for pull, it looks reasonable as to fix, I am not sure if it doesn't influences anything else, can you explain those two test changes?

@wishdev
Copy link
Copy Markdown
Author

wishdev commented Jun 23, 2025

My apologies - both of those tests failed originally for me as they crossed DST changes in North America. I had "commented" them out by switching the dates. That should have been changed back prior to the original push - corrected.

@exander77
Copy link
Copy Markdown
Owner

I think this breaks something else, I need to look into it deeper. Main issue is that underlying time implementation from libc is not good in general (both glibc and musl). So it is really hard to build on top of it.

@wishdev
Copy link
Copy Markdown
Author

wishdev commented Jul 11, 2025

I'm certainly not an expert so I can appreciate the caution here. The beauty is that I can move right along in my little world with my patch and be very happy - open source at its finest!

@exander77
Copy link
Copy Markdown
Owner

I will look into this more so see what can be done about DST changes across different timezones.

@slavslavov
Copy link
Copy Markdown

When I had the need to use timezones, local time did not work for me so I have implemented my own function to deal with time zones and handle DST and the the edge cases around changes. For this to work, I set local time to false in cmake
set(CRON_USE_LOCAL_TIME false)
And I use the date library to convert to the required timezone taking into account DST. Of course ambiguities have to be resolved and there is no standard way, even big cron libraries like Quartz, Kunermetes, AWS, Google handle this in different way so the best way is to chose what suits your needs and document it

When the user creates a cron string, they have to specify the timezone in which they want it executed. The function converts to UTC so one scheduler can handle different timezones.

date::sys_seconds nextCron(date::time_zone const & tz, date::sys_seconds baseTime, cron_expr * expr)
{
    date::local_seconds localBase = date::floor<std::chrono::seconds>(tz.to_local(baseTime));
    auto localBaseInfo = tz.get_info(localBase);

    time_t localNext = cron_next(expr, localBase.time_since_epoch().count());
    if (localNext <= 0)
        return date::sys_seconds{std::chrono::seconds{0}};
    date::local_seconds localNextChrono{std::chrono::seconds{localNext}};
    auto localNextInfo = tz.get_info(localNextChrono);
    auto baseInfo = tz.get_info(baseTime);

    switch (localNextInfo.result)
    {
        case date::local_info::nonexistent:
            return baseTime + (localNextChrono - localBase) + baseInfo.offset;
        case date::local_info::ambiguous:
            return tz.to_sys(localNextChrono, date::choose::earliest);
        case date::local_info::unique:
            return tz.to_sys(localNextChrono);
    }
    return date::sys_seconds{std::chrono::seconds{0}};
}

@exander77
Copy link
Copy Markdown
Owner

I implemented some DST and other fixes, I took some ideas from here.

  1. Added loop guard macros: CRON_MAX_DO_NEXTPREV_ITERS, CRON_MAX_CRON_ITERS. This should prevent infinite loops in some non-standard situations.
  2. Fixed find_day() exhaustion logic so impossible schedules correctly return CRON_INVALID_INSTANT, especially in strict mode (see below).
  3. Split scheduler flow into cron_once() + cron() wrapper.
  4. Added strict mode (CRON_STRICT_MATCH) with post-normalization calendar revalidation, monotonic progress checks, and retry loop guards. Default behavior is relaxed (legacy candidate return). Strict regime returns more CRON_INVALID_INSTANT for cases where normal (relaxed mode) would issue some option.
  5. In strict + local-time mode, DST is forced to recompute via tm_isdst = -1 and use timeline-based carry helper (roll_carry_field) for DST robustness.
  6. Added test-matrix and test-scan-timezones targets, test-matrix test build with cc/musl under various timezones, it should help with regression reintroduction in the future. It is really hard to foresee side-effects of certain changes.

@exander77
Copy link
Copy Markdown
Owner

I specifically added Toronto timezone in the matrix tests. Added DST tests from this pull.

I moved is_dst and some other fixes to relaxed mode as well, they should be there.

To run that timezone specifically:

TIMEZONES='America/Toronto' bash test/run_ccronexpr_test_matrix.sh

@exander77
Copy link
Copy Markdown
Owner

exander77 commented Mar 2, 2026

Improved musl compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants