Skip to content

Commit

Permalink
Obtain local offset in multi-threaded situations
Browse files Browse the repository at this point in the history
  • Loading branch information
jhpratt committed Dec 3, 2024
1 parent 1e19827 commit febf3a1
Show file tree
Hide file tree
Showing 12 changed files with 179 additions and 234 deletions.
2 changes: 0 additions & 2 deletions tests/derives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ fn clone() {
assert_cloned_eq!(well_known::iso8601::FormattedComponents::None);
assert_cloned_eq!(component_range_error());
assert_cloned_eq!(BorrowedFormatItem::Literal(b""));
assert_cloned_eq!(time::util::local_offset::Soundness::Sound);

assert_cloned_eq!(modifier::Day::default());
assert_cloned_eq!(modifier::MonthRepr::default());
Expand Down Expand Up @@ -166,7 +165,6 @@ fn debug() {
well_known::iso8601::Config::DEFAULT;
component_range_error();
Error::ConversionRange(ConversionRange);
time::util::local_offset::Soundness::Sound;

modifier::Day::default();
modifier::MonthRepr::default();
Expand Down
7 changes: 1 addition & 6 deletions tests/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![allow(
missing_docs,
clippy::missing_const_for_fn, // irrelevant for tests
clippy::std_instead_of_core, // irrelevant for tests
clippy::std_instead_of_alloc, // irrelevant for tests
Expand Down Expand Up @@ -71,12 +72,6 @@ macro_rules! require_all_features {
}

require_all_features! {
use std::sync::Mutex;

/// A lock to ensure that certain tests don't run in parallel, which could lead to a test
/// unexpectedly failing.
static SOUNDNESS_LOCK: Mutex<()> = Mutex::new(());

// Required by the crate for technical reasons.
#[allow(clippy::single_component_path_imports)]
use rstest_reuse;
Expand Down
101 changes: 35 additions & 66 deletions tests/offset_date_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,9 @@ fn now_utc() {
assert_eq!(OffsetDateTime::now_utc().offset(), offset!(UTC));
}

#[cfg_attr(miri, ignore)]
#[test]
fn now_local() {
use time::util::local_offset::*;

let _guard = crate::SOUNDNESS_LOCK.lock().expect("lock is poisoned");

// Safety: Technically not sound. However, this is a test, and it's highly improbable that we
// will run into issues with setting an environment variable a few times.
unsafe { set_soundness(Soundness::Unsound) };
assert!(OffsetDateTime::now_local().is_ok());
// Safety: We're setting it back to sound.
unsafe { set_soundness(Soundness::Sound) };
}

#[test]
Expand Down Expand Up @@ -450,16 +440,12 @@ fn replace_year() {
datetime!(2022 - 02 - 18 12:00 +01).replace_year(2019),
Ok(datetime!(2019 - 02 - 18 12:00 +01))
);
assert!(
datetime!(2022 - 02 - 18 12:00 +01)
.replace_year(-1_000_000_000)
.is_err()
); // -1_000_000_000 isn't a valid year
assert!(
datetime!(2022 - 02 - 18 12:00 +01)
.replace_year(1_000_000_000)
.is_err()
); // 1_000_000_000 isn't a valid year
assert!(datetime!(2022 - 02 - 18 12:00 +01)
.replace_year(-1_000_000_000)
.is_err()); // -1_000_000_000 isn't a valid year
assert!(datetime!(2022 - 02 - 18 12:00 +01)
.replace_year(1_000_000_000)
.is_err()); // 1_000_000_000 isn't a valid year
}

#[test]
Expand All @@ -468,11 +454,9 @@ fn replace_month() {
datetime!(2022 - 02 - 18 12:00 +01).replace_month(Month::January),
Ok(datetime!(2022 - 01 - 18 12:00 +01))
);
assert!(
datetime!(2022 - 01 - 30 12:00 +01)
.replace_month(Month::February)
.is_err()
); // 30 isn't a valid day in February
assert!(datetime!(2022 - 01 - 30 12:00 +01)
.replace_month(Month::February)
.is_err()); // 30 isn't a valid day in February
}

#[test]
Expand All @@ -482,7 +466,8 @@ fn replace_day() {
Ok(datetime!(2022 - 02 - 01 12:00 +01))
);
assert!(datetime!(2022 - 02 - 18 12:00 +01).replace_day(0).is_err()); // 00 isn't a valid day
assert!(datetime!(2022 - 02 - 18 12:00 +01).replace_day(30).is_err()); // 30 isn't a valid day in February
assert!(datetime!(2022 - 02 - 18 12:00 +01).replace_day(30).is_err()); // 30 isn't a valid day
// in February
}

#[test]
Expand All @@ -496,16 +481,12 @@ fn replace_ordinal() {
Ok(datetime!(2024 - 366 12:00 +01))
);
assert!(datetime!(2022 - 049 12:00 +01).replace_ordinal(0).is_err()); // 0 isn't a valid day
assert!(
datetime!(2022 - 049 12:00 +01)
.replace_ordinal(366)
.is_err()
); // 2022 isn't a leap year
assert!(
datetime!(2022 - 049 12:00 +01)
.replace_ordinal(367)
.is_err()
); // 367 isn't a valid day
assert!(datetime!(2022 - 049 12:00 +01)
.replace_ordinal(366)
.is_err()); // 2022 isn't a leap year
assert!(datetime!(2022 - 049 12:00 +01)
.replace_ordinal(367)
.is_err()); // 367 isn't a valid day
}

#[test]
Expand All @@ -514,11 +495,9 @@ fn replace_hour() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_hour(7),
Ok(datetime!(2022 - 02 - 18 07:02:03.004_005_006 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_hour(24)
.is_err()
); // 24 isn't a valid hour
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_hour(24)
.is_err()); // 24 isn't a valid hour
}

#[test]
Expand All @@ -527,11 +506,9 @@ fn replace_minute() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_minute(7),
Ok(datetime!(2022 - 02 - 18 01:07:03.004_005_006 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_minute(60)
.is_err()
); // 60 isn't a valid minute
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_minute(60)
.is_err()); // 60 isn't a valid minute
}

#[test]
Expand All @@ -540,11 +517,9 @@ fn replace_second() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_second(7),
Ok(datetime!(2022 - 02 - 18 01:02:07.004_005_006 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_second(60)
.is_err()
); // 60 isn't a valid second
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_second(60)
.is_err()); // 60 isn't a valid second
}

#[test]
Expand All @@ -553,11 +528,9 @@ fn replace_millisecond() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_millisecond(7),
Ok(datetime!(2022 - 02 - 18 01:02:03.007 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_millisecond(1_000)
.is_err()
); // 1_000 isn't a valid millisecond
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_millisecond(1_000)
.is_err()); // 1_000 isn't a valid millisecond
}

#[test]
Expand All @@ -566,11 +539,9 @@ fn replace_microsecond() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_microsecond(7_008),
Ok(datetime!(2022 - 02 - 18 01:02:03.007_008 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_microsecond(1_000_000)
.is_err()
); // 1_000_000 isn't a valid microsecond
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_microsecond(1_000_000)
.is_err()); // 1_000_000 isn't a valid microsecond
}

#[test]
Expand All @@ -579,11 +550,9 @@ fn replace_nanosecond() {
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01).replace_nanosecond(7_008_009),
Ok(datetime!(2022 - 02 - 18 01:02:03.007_008_009 +01))
);
assert!(
datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_nanosecond(1_000_000_000)
.is_err()
); // 1_000_000_000 isn't a valid nanosecond
assert!(datetime!(2022 - 02 - 18 01:02:03.004_005_006 +01)
.replace_nanosecond(1_000_000_000)
.is_err()); // 1_000_000_000 isn't a valid nanosecond
}

#[test]
Expand Down
35 changes: 2 additions & 33 deletions tests/utc_offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,51 +155,20 @@ fn neg(#[case] offset: UtcOffset, #[case] expected: UtcOffset) {
assert_eq!(-offset, expected);
}

#[cfg_attr(miri, ignore)]
#[test]
fn local_offset_at() {
use time::util::local_offset::*;

let _guard = crate::SOUNDNESS_LOCK.lock().expect("lock is poisoned");

// Safety: Technically not sound. However, this is a test, and it's highly improbable that we
// will run into issues with setting an environment variable a few times.
unsafe { set_soundness(Soundness::Unsound) };
assert!(UtcOffset::local_offset_at(OffsetDateTime::UNIX_EPOCH).is_ok());
// Safety: We're setting it back to sound.
unsafe { set_soundness(Soundness::Sound) };
}

#[cfg_attr(miri, ignore)]
#[test]
fn current_local_offset() {
use time::util::local_offset::*;

let _guard = crate::SOUNDNESS_LOCK.lock().expect("lock is poisoned");

// Safety: Technically not sound. However, this is a test, and it's highly improbable that we
// will run into issues with setting an environment variable a few times.
unsafe { set_soundness(Soundness::Unsound) };
assert!(UtcOffset::current_local_offset().is_ok());
// Safety: We're setting it back to sound.
unsafe { set_soundness(Soundness::Sound) };
}

// Note: This behavior is not guaranteed and will hopefully be changed in the future.
#[test]
#[cfg_attr(
any(
target_os = "netbsd",
target_os = "illumos",
not(target_family = "unix")
),
ignore
)]
fn local_offset_error_when_multithreaded() {
let _guard = crate::SOUNDNESS_LOCK.lock().expect("lock is poisoned");

fn local_offset_success_when_multithreaded() {
std::thread::spawn(|| {
assert!(UtcOffset::current_local_offset().is_err());
assert!(UtcOffset::current_local_offset().is_ok());
})
.join()
.expect("failed to join thread");
Expand Down
11 changes: 5 additions & 6 deletions tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,16 @@ fn weeks_in_year() {
}

#[rstest]
#[allow(deprecated)]
fn local_offset_soundness() {
use time::util::local_offset::*;

let _guard = crate::SOUNDNESS_LOCK.lock().expect("lock is poisoned");

// These functions no longer do anything so they always return `Sound`.
assert_eq!(get_soundness(), Soundness::Sound);
// Safety: Technically not sound. However, this is a test, and it's highly improbable that we
// will run into issues with setting an environment variable a few times.
// Safety: This no longer has any safety requirements.
unsafe { set_soundness(Soundness::Unsound) };
assert_eq!(get_soundness(), Soundness::Unsound);
// Safety: We're setting it back to sound.
assert_eq!(get_soundness(), Soundness::Sound);
// Safety: See above.
unsafe { set_soundness(Soundness::Sound) };
assert_eq!(get_soundness(), Soundness::Sound);
}
7 changes: 1 addition & 6 deletions time/src/sys/local_offset_at/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,5 @@ use crate::{OffsetDateTime, UtcOffset};
/// Attempt to obtain the system's UTC offset. If the offset cannot be determined, `None` is
/// returned.
pub(crate) fn local_offset_at(datetime: OffsetDateTime) -> Option<UtcOffset> {
// miri does not support tzset()
if cfg!(miri) {
None
} else {
imp::local_offset_at(datetime)
}
imp::local_offset_at(datetime)
}
61 changes: 4 additions & 57 deletions time/src/sys/local_offset_at/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,16 @@
use core::mem::MaybeUninit;

use crate::util::local_offset::{self, Soundness};
use crate::{OffsetDateTime, UtcOffset};

/// Whether the operating system has a thread-safe environment. This allows bypassing the check for
/// if the process is multi-threaded.
// This is the same value as `cfg!(target_os = "x")`.
// Use byte-strings to work around current limitations of const eval.
const OS_HAS_THREAD_SAFE_ENVIRONMENT: bool = match std::env::consts::OS.as_bytes() {
// https://github.com/illumos/illumos-gate/blob/0fb96ba1f1ce26ff8b286f8f928769a6afcb00a6/usr/src/lib/libc/port/gen/getenv.c
b"illumos"
// https://github.com/NetBSD/src/blob/f45028636a44111bc4af44d460924958a4460844/lib/libc/stdlib/getenv.c
// https://github.com/NetBSD/src/blob/f45028636a44111bc4af44d460924958a4460844/lib/libc/stdlib/setenv.c
| b"netbsd"
=> true,
_ => false,
};

/// Convert the given Unix timestamp to a `libc::tm`. Returns `None` on any error.
///
/// # Safety
///
/// This method must only be called when the process is single-threaded.
///
/// This method will remain `unsafe` until `std::env::set_var` is deprecated or has its behavior
/// altered. This method is, on its own, safe. It is the presence of a safe, unsound way to set
/// environment variables that makes it unsafe.
unsafe fn timestamp_to_tm(timestamp: i64) -> Option<libc::tm> {
extern "C" {
#[cfg_attr(target_os = "netbsd", link_name = "__tzset50")]
fn tzset();
}

fn timestamp_to_tm(timestamp: i64) -> Option<libc::tm> {
// The exact type of `timestamp` beforehand can vary, so this conversion is necessary.
#[allow(clippy::useless_conversion)]
let timestamp = timestamp.try_into().ok()?;

let mut tm = MaybeUninit::uninit();

// Update timezone information from system. `localtime_r` does not do this for us.
//
// Safety: tzset is thread-safe.
unsafe { tzset() };

// Safety: We are calling a system API, which mutates the `tm` variable. If a null
// pointer is returned, an error occurred.
let tm_ptr = unsafe { libc::localtime_r(&timestamp, tm.as_mut_ptr()) };
Expand Down Expand Up @@ -128,27 +95,7 @@ fn tm_to_offset(unix_timestamp: i64, tm: libc::tm) -> Option<UtcOffset> {

/// Obtain the system's UTC offset.
pub(super) fn local_offset_at(datetime: OffsetDateTime) -> Option<UtcOffset> {
// Continue to obtaining the UTC offset if and only if the call is sound or the user has
// explicitly opted out of soundness.
//
// Soundness can be guaranteed either by knowledge of the operating system or knowledge that the
// process is single-threaded. If the process is single-threaded, then the environment cannot
// be mutated by a different thread in the process while execution of this function is taking
// place, which can cause a segmentation fault by dereferencing a dangling pointer.
//
// If the `num_threads` crate is incapable of determining the number of running threads, then
// we conservatively return `None` to avoid a soundness bug.

if OS_HAS_THREAD_SAFE_ENVIRONMENT
|| local_offset::get_soundness() == Soundness::Unsound
|| num_threads::is_single_threaded() == Some(true)
{
let unix_timestamp = datetime.unix_timestamp();
// Safety: We have just confirmed that the process is single-threaded or the user has
// explicitly opted out of soundness.
let tm = unsafe { timestamp_to_tm(unix_timestamp) }?;
tm_to_offset(unix_timestamp, tm)
} else {
None
}
let unix_timestamp = datetime.unix_timestamp();
let tm = timestamp_to_tm(unix_timestamp)?;
tm_to_offset(unix_timestamp, tm)
}
Loading

0 comments on commit febf3a1

Please sign in to comment.