Determination of local timestamp is not consistent when the date time is ambiguous on Linux

[x-hgg-x]

The following test fails on glibc Linux:

#[test]
fn test() {
    use chrono::offset::local::datetime_to_timespec;

    std::env::set_var("TZ", "Europe/Paris");

    let date = NaiveDate::from_ymd(2021, 10, 31);

    let date_time_1_30 = date.and_hms(1, 30, 0);
    let date_time_2_00 = date.and_hms(2, 0, 0);
    let date_time_2_30 = date.and_hms(2, 30, 0);
    let date_time_3_00 = date.and_hms(3, 0, 0);
    let date_time_3_30 = date.and_hms(3, 30, 0);

    let timestamp_1_30_a = datetime_to_timespec(&date_time_1_30, true).sec; // 1635636600
    let timestamp_2_00_a = datetime_to_timespec(&date_time_2_00, true).sec; // 1635638400
    let timestamp_2_30_a = datetime_to_timespec(&date_time_2_30, true).sec; // 1635640200
    let timestamp_3_00_a = datetime_to_timespec(&date_time_3_00, true).sec; // 1635645600

    let _timestamp_3_30_ = datetime_to_timespec(&date_time_3_30, true).sec; // 1635647400

    let timestamp_3_00_b = datetime_to_timespec(&date_time_3_00, true).sec; // 1635645600
    let timestamp_2_30_b = datetime_to_timespec(&date_time_2_30, true).sec; // 1635643800
    let timestamp_2_00_b = datetime_to_timespec(&date_time_2_00, true).sec; // 1635642000
    let timestamp_1_30_b = datetime_to_timespec(&date_time_1_30, true).sec; // 1635636600

    // Passing asserts
    assert_eq!(timestamp_1_30_a, timestamp_1_30_b);
    assert_eq!(timestamp_3_00_a, timestamp_3_00_b);

    // Failing asserts
    assert_eq!(timestamp_2_00_a, timestamp_2_00_b);
    assert_eq!(timestamp_2_30_a, timestamp_2_30_b);
}

This is because this function calls libc::mktime with tm_isdst = -1 for automatically determining DST, which depends on global context (modified by each call of libc::mktime).

The same behavior is observed in C: see Godbolt.

[djc]

Thanks for reporting! Going to look into this soon.

[x-hgg-x]

I searched in the glibc source and found this line, where glibc stores the previous mktime offset in a static long int variable.

Note that this variable doesn't seem to be associated with any locks, even if the function should be MT-Safe env locale (see attributes description), so I don't know if the mktime function can be safely used in a multithread program, even if you don't modify any environment variables.

Edit: there is at least a data-race when using the mktime function in a multithread context, as shown in the following log from valgrind --tool=helgrind with a debug build of glibc:

==912504== 
==912504== Possible data race during write of size 8 at 0x4A24398 by thread #2
==912504== Locks held: none
==912504==    at 0x48F2220: __mktime_internal (mktime.c:495)
==912504==    by 0x4802821: test (test.c:20)
==912504==    by 0x48B3407: start_thread (pthread_create.c:439)
==912504==    by 0x4935383: clone (clone.S:100)
==912504== 
==912504== This conflicts with a previous write of size 8 by thread #1
==912504== Locks held: none
==912504==    at 0x48F2220: __mktime_internal (mktime.c:495)
==912504==    by 0x4802821: test (test.c:20)
==912504==    by 0x480266E: main (test.c:30)
==912504==  Address 0x4a24398 is 0 bytes inside data symbol "localtime_offset.4897"
==912504== 

[pitdicker]

I updated the test to run with the current version of chrono, 0.4.x.

#[test]
fn issue_668() {
    std::env::set_var("TZ", "Europe/Paris");

    let date = NaiveDate::from_ymd_opt(2021, 10, 31).unwrap();

    let date_time_1_30 = date.and_hms(1, 30, 0).and_local_timezone(Local).unwrap();
    let date_time_2_00 = date.and_hms(2, 0, 0).and_local_timezone(Local).unwrap();
    let date_time_2_30 = date.and_hms(2, 30, 0).and_local_timezone(Local).unwrap();
    let date_time_3_00 = date.and_hms(3, 0, 0).and_local_timezone(Local).unwrap();
    let date_time_3_30 = date.and_hms(3, 30, 0).and_local_timezone(Local).unwrap();

    let timestamp_1_30_a = date_time_1_30.timestamp(); // 1635636600
    let timestamp_2_00_a = date_time_2_00.timestamp(); // 1635638400
    let timestamp_2_30_a = date_time_2_30.timestamp(); // 1635640200
    let timestamp_3_00_a = date_time_3_00.timestamp(); // 1635645600

    let _timestamp_3_30_ = date_time_3_30.timestamp(); // 1635647400

    let timestamp_3_00_b = date_time_3_00.timestamp(); // 1635645600
    let timestamp_2_30_b = date_time_2_30.timestamp(); // 1635643800
    let timestamp_2_00_b = date_time_2_00.timestamp(); // 1635642000
    let timestamp_1_30_b = date_time_1_30.timestamp(); // 1635636600

    // Passing asserts
    assert_eq!(timestamp_1_30_a, timestamp_1_30_b);
    assert_eq!(timestamp_3_00_a, timestamp_3_00_b);

    // Failing asserts
    assert_eq!(timestamp_2_00_a, timestamp_2_00_b);
    assert_eq!(timestamp_2_30_a, timestamp_2_30_b);
}

It fails with:

---- datetime::tests::issue_668 stdout ----
thread 'datetime::tests::issue_668' panicked at 'Ambiguous local time, ranging from 2021-10-31T02:00:00+01:00 to 2021-10-31T02:00:00+02:00', src/offset/mod.rs:199:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I would say this is correct. It panics because in this test I didn't deal with the ambiguity of a local time during a DST transition, but unwrapped the result.

Given how the implementation has changed, I don't believe this issue exists anymore.

[jtmoon79]

I updated the test to run with the current version of chrono, 0.4.x.

@pitdicker can you link to that PR?

[pitdicker]

There is no PR, just the comment. Is it worth adding this as a test? Is seems to me like we are not short on tests for timezone transitions.

[jtmoon79]

Is it worth adding this as a test?

This still fails in the current public release, so I lean toward "yes". If you don't get to it then I will.