fix: Use SystemTime instead of Instant everywhere

If a time value doesn't need to be sent to another host, saved to the db or otherwise used across
program restarts, a monotonically nondecreasing clock (`Instant`) should be used. But as `Instant`
may use `libc::clock_gettime(CLOCK_MONOTONIC)`, e.g. on Android, and does not advance while being in
deep sleep mode, get rid of `Instant` in favor of using `SystemTime`, but add `tools::Time` as an
alias for it with the appropriate comment so that it's clear why `Instant` isn't used in those
places and to protect from unwanted usages of `Instant` in the future. Also this can help to switch
to another clock impl if we find any.
This commit is contained in:
iequidoo
2023-12-14 20:42:25 -03:00
committed by iequidoo
parent f4ed63c54c
commit 31ee3feb57
10 changed files with 56 additions and 45 deletions

View File

@@ -217,7 +217,7 @@ pub(crate) const DC_FOLDERS_CONFIGURED_VERSION: i32 = 4;
pub(crate) const DEFAULT_MAX_SMTP_RCPT_TO: usize = 50; pub(crate) const DEFAULT_MAX_SMTP_RCPT_TO: usize = 50;
/// How far the last quota check needs to be in the past to be checked by the background function (in seconds). /// How far the last quota check needs to be in the past to be checked by the background function (in seconds).
pub(crate) const DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT: i64 = 12 * 60 * 60; // 12 hours pub(crate) const DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT: u64 = 12 * 60 * 60; // 12 hours
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {

View File

@@ -6,7 +6,7 @@ use std::ops::Deref;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc; use std::sync::Arc;
use std::time::{Duration, Instant, SystemTime}; use std::time::Duration;
use anyhow::{bail, ensure, Context as _, Result}; use anyhow::{bail, ensure, Context as _, Result};
use async_channel::{self as channel, Receiver, Sender}; use async_channel::{self as channel, Receiver, Sender};
@@ -33,7 +33,7 @@ use crate::scheduler::{convert_folder_meaning, SchedulerState};
use crate::sql::Sql; use crate::sql::Sql;
use crate::stock_str::StockStrings; use crate::stock_str::StockStrings;
use crate::timesmearing::SmearedTimestamp; use crate::timesmearing::SmearedTimestamp;
use crate::tools::{create_id, duration_to_str, time}; use crate::tools::{self, create_id, duration_to_str, time, time_elapsed};
/// Builder for the [`Context`]. /// Builder for the [`Context`].
/// ///
@@ -233,7 +233,7 @@ pub struct InnerContext {
/// IMAP METADATA. /// IMAP METADATA.
pub(crate) metadata: RwLock<Option<ServerMetadata>>, pub(crate) metadata: RwLock<Option<ServerMetadata>>,
pub(crate) last_full_folder_scan: Mutex<Option<Instant>>, pub(crate) last_full_folder_scan: Mutex<Option<tools::Time>>,
/// ID for this `Context` in the current process. /// ID for this `Context` in the current process.
/// ///
@@ -241,7 +241,7 @@ pub struct InnerContext {
/// be identified by this ID. /// be identified by this ID.
pub(crate) id: u32, pub(crate) id: u32,
creation_time: SystemTime, creation_time: tools::Time,
/// The text of the last error logged and emitted as an event. /// The text of the last error logged and emitted as an event.
/// If the ui wants to display an error after a failure, /// If the ui wants to display an error after a failure,
@@ -262,7 +262,7 @@ enum RunningState {
Running { cancel_sender: Sender<()> }, Running { cancel_sender: Sender<()> },
/// Cancel signal has been sent, waiting for ongoing process to be freed. /// Cancel signal has been sent, waiting for ongoing process to be freed.
ShallStop { request: Instant }, ShallStop { request: tools::Time },
/// There is no ongoing process, a new one can be allocated. /// There is no ongoing process, a new one can be allocated.
Stopped, Stopped,
@@ -394,7 +394,7 @@ impl Context {
new_msgs_notify, new_msgs_notify,
server_id: RwLock::new(None), server_id: RwLock::new(None),
metadata: RwLock::new(None), metadata: RwLock::new(None),
creation_time: std::time::SystemTime::now(), creation_time: tools::Time::now(),
last_full_folder_scan: Mutex::new(None), last_full_folder_scan: Mutex::new(None),
last_error: std::sync::RwLock::new("".to_string()), last_error: std::sync::RwLock::new("".to_string()),
debug_logging: std::sync::RwLock::new(None), debug_logging: std::sync::RwLock::new(None),
@@ -454,7 +454,7 @@ impl Context {
} }
let address = self.get_primary_self_addr().await?; let address = self.get_primary_self_addr().await?;
let time_start = std::time::SystemTime::now(); let time_start = tools::Time::now();
info!(self, "background_fetch started fetching {address}"); info!(self, "background_fetch started fetching {address}");
let _pause_guard = self.scheduler.pause(self.clone()).await?; let _pause_guard = self.scheduler.pause(self.clone()).await?;
@@ -476,7 +476,10 @@ impl Context {
let quota = self.quota.read().await; let quota = self.quota.read().await;
quota quota
.as_ref() .as_ref()
.filter(|quota| quota.modified + DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT > time()) .filter(|quota| {
time_elapsed(&quota.modified)
> Duration::from_secs(DC_BACKGROUND_FETCH_QUOTA_CHECK_RATELIMIT)
})
.is_none() .is_none()
}; };
@@ -489,7 +492,7 @@ impl Context {
info!( info!(
self, self,
"background_fetch done for {address} took {:?}", "background_fetch done for {address} took {:?}",
time_start.elapsed().unwrap_or_default() time_elapsed(&time_start),
); );
Ok(()) Ok(())
@@ -591,7 +594,7 @@ impl Context {
pub(crate) async fn free_ongoing(&self) { pub(crate) async fn free_ongoing(&self) {
let mut s = self.running_state.write().await; let mut s = self.running_state.write().await;
if let RunningState::ShallStop { request } = *s { if let RunningState::ShallStop { request } = *s {
info!(self, "Ongoing stopped in {:?}", request.elapsed()); info!(self, "Ongoing stopped in {:?}", time_elapsed(&request));
} }
*s = RunningState::Stopped; *s = RunningState::Stopped;
} }
@@ -606,7 +609,7 @@ impl Context {
} }
info!(self, "Signaling the ongoing process to stop ASAP.",); info!(self, "Signaling the ongoing process to stop ASAP.",);
*s = RunningState::ShallStop { *s = RunningState::ShallStop {
request: Instant::now(), request: tools::Time::now(),
}; };
} }
RunningState::ShallStop { .. } | RunningState::Stopped => { RunningState::ShallStop { .. } | RunningState::Stopped => {
@@ -867,8 +870,8 @@ impl Context {
.to_string(), .to_string(),
); );
let elapsed = self.creation_time.elapsed(); let elapsed = time_elapsed(&self.creation_time);
res.insert("uptime", duration_to_str(elapsed.unwrap_or_default())); res.insert("uptime", duration_to_str(elapsed));
Ok(res) Ok(res)
} }
@@ -1214,7 +1217,7 @@ pub fn get_version_str() -> &'static str {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::time::Duration; use std::time::{Duration, SystemTime};
use anyhow::Context as _; use anyhow::Context as _;
use strum::IntoEnumIterator; use strum::IntoEnumIterator;

View File

@@ -1,4 +1,4 @@
use std::time::{Duration, SystemTime}; use std::time::Duration;
use anyhow::{bail, Context as _, Result}; use anyhow::{bail, Context as _, Result};
use async_channel::Receiver; use async_channel::Receiver;
@@ -11,6 +11,7 @@ use crate::config::Config;
use crate::context::Context; use crate::context::Context;
use crate::imap::{client::IMAP_TIMEOUT, FolderMeaning}; use crate::imap::{client::IMAP_TIMEOUT, FolderMeaning};
use crate::log::LogExt; use crate::log::LogExt;
use crate::tools::{self, time_elapsed};
/// Timeout after which IDLE is finished /// Timeout after which IDLE is finished
/// if there are no responses from the server. /// if there are no responses from the server.
@@ -106,7 +107,7 @@ impl Imap {
// Idle using polling. This is also needed if we're not yet configured - // Idle using polling. This is also needed if we're not yet configured -
// in this case, we're waiting for a configure job (and an interrupt). // in this case, we're waiting for a configure job (and an interrupt).
let fake_idle_start_time = SystemTime::now(); let fake_idle_start_time = tools::Time::now();
// Do not poll, just wait for an interrupt when no folder is passed in. // Do not poll, just wait for an interrupt when no folder is passed in.
let watch_folder = if let Some(watch_folder) = watch_folder { let watch_folder = if let Some(watch_folder) = watch_folder {
@@ -196,11 +197,7 @@ impl Imap {
info!( info!(
context, context,
"IMAP-fake-IDLE done after {:.4}s", "IMAP-fake-IDLE done after {:.4}s",
SystemTime::now() time_elapsed(&fake_idle_start_time).as_millis() as f64 / 1000.,
.duration_since(fake_idle_start_time)
.unwrap_or_default()
.as_millis() as f64
/ 1000.,
); );
} }
} }

View File

@@ -1,4 +1,4 @@
use std::{collections::BTreeMap, time::Instant}; use std::collections::BTreeMap;
use anyhow::{Context as _, Result}; use anyhow::{Context as _, Result};
use futures::TryStreamExt; use futures::TryStreamExt;
@@ -7,6 +7,7 @@ use super::{get_folder_meaning_by_attrs, get_folder_meaning_by_name};
use crate::config::Config; use crate::config::Config;
use crate::imap::Imap; use crate::imap::Imap;
use crate::log::LogExt; use crate::log::LogExt;
use crate::tools::{self, time_elapsed};
use crate::{context::Context, imap::FolderMeaning}; use crate::{context::Context, imap::FolderMeaning};
impl Imap { impl Imap {
@@ -15,7 +16,7 @@ impl Imap {
// First of all, debounce to once per minute: // First of all, debounce to once per minute:
let mut last_scan = context.last_full_folder_scan.lock().await; let mut last_scan = context.last_full_folder_scan.lock().await;
if let Some(last_scan) = *last_scan { if let Some(last_scan) = *last_scan {
let elapsed_secs = last_scan.elapsed().as_secs(); let elapsed_secs = time_elapsed(&last_scan).as_secs();
let debounce_secs = context let debounce_secs = context
.get_config_u64(Config::ScanAllFoldersDebounceSecs) .get_config_u64(Config::ScanAllFoldersDebounceSecs)
.await?; .await?;
@@ -93,7 +94,7 @@ impl Imap {
.await?; .await?;
} }
last_scan.replace(Instant::now()); last_scan.replace(tools::Time::now());
Ok(true) Ok(true)
} }

View File

@@ -18,7 +18,7 @@ use crate::constants::KeyGenType;
use crate::context::Context; use crate::context::Context;
use crate::log::LogExt; use crate::log::LogExt;
use crate::pgp::KeyPair; use crate::pgp::KeyPair;
use crate::tools::EmailAddress; use crate::tools::{self, time_elapsed, EmailAddress};
/// Convenience trait for working with keys. /// Convenience trait for working with keys.
/// ///
@@ -204,7 +204,7 @@ async fn generate_keypair(context: &Context) -> Result<KeyPair> {
match load_keypair(context, &addr).await? { match load_keypair(context, &addr).await? {
Some(key_pair) => Ok(key_pair), Some(key_pair) => Ok(key_pair),
None => { None => {
let start = std::time::SystemTime::now(); let start = tools::Time::now();
let keytype = KeyGenType::from_i32(context.get_config_int(Config::KeyGenType).await?) let keytype = KeyGenType::from_i32(context.get_config_int(Config::KeyGenType).await?)
.unwrap_or_default(); .unwrap_or_default();
info!(context, "Generating keypair with type {}", keytype); info!(context, "Generating keypair with type {}", keytype);
@@ -216,7 +216,7 @@ async fn generate_keypair(context: &Context) -> Result<KeyPair> {
info!( info!(
context, context,
"Keypair generated in {:.3}s.", "Keypair generated in {:.3}s.",
start.elapsed().unwrap_or_default().as_secs() time_elapsed(&start).as_secs(),
); );
Ok(keypair) Ok(keypair)
} }

View File

@@ -12,7 +12,7 @@ use crate::imap::scan_folders::get_watched_folders;
use crate::imap::session::Session as ImapSession; use crate::imap::session::Session as ImapSession;
use crate::imap::Imap; use crate::imap::Imap;
use crate::message::{Message, Viewtype}; use crate::message::{Message, Viewtype};
use crate::tools::time; use crate::tools;
use crate::{stock_str, EventType}; use crate::{stock_str, EventType};
/// warn about a nearly full mailbox after this usage percentage is reached. /// warn about a nearly full mailbox after this usage percentage is reached.
@@ -40,8 +40,8 @@ pub struct QuotaInfo {
/// set to `Ok()` for valid quota information. /// set to `Ok()` for valid quota information.
pub(crate) recent: Result<BTreeMap<String, Vec<QuotaResource>>>, pub(crate) recent: Result<BTreeMap<String, Vec<QuotaResource>>>,
/// Timestamp when structure was modified. /// When the structure was modified.
pub(crate) modified: i64, pub(crate) modified: tools::Time,
} }
async fn get_unique_quota_roots_and_usage( async fn get_unique_quota_roots_and_usage(
@@ -151,7 +151,7 @@ impl Context {
*self.quota.write().await = Some(QuotaInfo { *self.quota.write().await = Some(QuotaInfo {
recent: quota, recent: quota,
modified: time(), modified: tools::Time::now(),
}); });
self.emit_event(EventType::ConnectivityChanged); self.emit_event(EventType::ConnectivityChanged);

View File

@@ -2,6 +2,7 @@ use std::cmp;
use std::iter::{self, once}; use std::iter::{self, once};
use std::num::NonZeroUsize; use std::num::NonZeroUsize;
use std::sync::atomic::Ordering; use std::sync::atomic::Ordering;
use std::time::Duration;
use anyhow::{bail, Context as _, Error, Result}; use anyhow::{bail, Context as _, Error, Result};
use async_channel::{self as channel, Receiver, Sender}; use async_channel::{self as channel, Receiver, Sender};
@@ -24,7 +25,7 @@ use crate::log::LogExt;
use crate::message::MsgId; use crate::message::MsgId;
use crate::smtp::{send_smtp_messages, Smtp}; use crate::smtp::{send_smtp_messages, Smtp};
use crate::sql; use crate::sql;
use crate::tools::{duration_to_str, maybe_add_time_based_warnings, time}; use crate::tools::{self, duration_to_str, maybe_add_time_based_warnings, time, time_elapsed};
pub(crate) mod connectivity; pub(crate) mod connectivity;
@@ -398,7 +399,7 @@ async fn inbox_loop(
let quota = ctx.quota.read().await; let quota = ctx.quota.read().await;
quota quota
.as_ref() .as_ref()
.filter(|quota| quota.modified + 60 > time()) .filter(|quota| time_elapsed(&quota.modified) > Duration::from_secs(60))
.is_none() .is_none()
}; };
@@ -785,7 +786,7 @@ async fn smtp_loop(
// again, we increase the timeout exponentially, in order not to do lots of // again, we increase the timeout exponentially, in order not to do lots of
// unnecessary retries. // unnecessary retries.
if let Some(t) = timeout { if let Some(t) = timeout {
let now = tokio::time::Instant::now(); let now = tools::Time::now();
info!( info!(
ctx, ctx,
"smtp has messages to retry, planning to retry {} seconds later", t, "smtp has messages to retry, planning to retry {} seconds later", t,
@@ -796,7 +797,7 @@ async fn smtp_loop(
}) })
.await .await
.unwrap_or_default(); .unwrap_or_default();
let slept = (tokio::time::Instant::now() - now).as_secs(); let slept = time_elapsed(&now).as_secs();
timeout = Some(cmp::max( timeout = Some(cmp::max(
t, t,
slept.saturating_add(rand::thread_rng().gen_range((slept / 2)..=slept)), slept.saturating_add(rand::thread_rng().gen_range((slept / 2)..=slept)),

View File

@@ -2,7 +2,7 @@
pub mod send; pub mod send;
use std::time::{Duration, SystemTime}; use std::time::Duration;
use anyhow::{bail, format_err, Context as _, Error, Result}; use anyhow::{bail, format_err, Context as _, Error, Result};
use async_smtp::response::{Category, Code, Detail}; use async_smtp::response::{Category, Code, Detail};
@@ -28,6 +28,7 @@ use crate::scheduler::connectivity::ConnectivityStore;
use crate::socks::Socks5Config; use crate::socks::Socks5Config;
use crate::sql; use crate::sql;
use crate::stock_str::unencrypted_email; use crate::stock_str::unencrypted_email;
use crate::tools::{self, time_elapsed};
/// SMTP connection, write and read timeout. /// SMTP connection, write and read timeout.
const SMTP_TIMEOUT: Duration = Duration::from_secs(60); const SMTP_TIMEOUT: Duration = Duration::from_secs(60);
@@ -43,7 +44,7 @@ pub(crate) struct Smtp {
/// Timestamp of last successful send/receive network interaction /// Timestamp of last successful send/receive network interaction
/// (eg connect or send succeeded). On initialization and disconnect /// (eg connect or send succeeded). On initialization and disconnect
/// it is set to None. /// it is set to None.
last_success: Option<SystemTime>, last_success: Option<tools::Time>,
pub(crate) connectivity: ConnectivityStore, pub(crate) connectivity: ConnectivityStore,
@@ -72,11 +73,7 @@ impl Smtp {
/// have been successfully used the last 60 seconds /// have been successfully used the last 60 seconds
pub fn has_maybe_stale_connection(&self) -> bool { pub fn has_maybe_stale_connection(&self) -> bool {
if let Some(last_success) = self.last_success { if let Some(last_success) = self.last_success {
SystemTime::now() time_elapsed(&last_success).as_secs() > 60
.duration_since(last_success)
.unwrap_or_default()
.as_secs()
> 60
} else { } else {
false false
} }
@@ -336,7 +333,7 @@ impl Smtp {
} }
self.transport = Some(transport); self.transport = Some(transport);
self.last_success = Some(SystemTime::now()); self.last_success = Some(tools::Time::now());
context.emit_event(EventType::SmtpConnected(format!( context.emit_event(EventType::SmtpConnected(format!(
"SMTP-LOGIN as {} ok", "SMTP-LOGIN as {} ok",

View File

@@ -6,6 +6,7 @@ use super::Smtp;
use crate::config::Config; use crate::config::Config;
use crate::context::Context; use crate::context::Context;
use crate::events::EventType; use crate::events::EventType;
use crate::tools;
pub type Result<T> = std::result::Result<T, Error>; pub type Result<T> = std::result::Result<T, Error>;
@@ -55,7 +56,7 @@ impl Smtp {
format!("Message len={message_len_bytes} was SMTP-sent to {recipients_display}"); format!("Message len={message_len_bytes} was SMTP-sent to {recipients_display}");
info!(context, "{info_msg}."); info!(context, "{info_msg}.");
context.emit_event(EventType::SmtpMessageSent(info_msg)); context.emit_event(EventType::SmtpMessageSent(info_msg));
self.last_success = Some(std::time::SystemTime::now()); self.last_success = Some(tools::Time::now());
} else { } else {
warn!( warn!(
context, context,

View File

@@ -9,6 +9,13 @@ use std::io::{Cursor, Write};
use std::mem; use std::mem;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::str::from_utf8; use std::str::from_utf8;
// If a time value doesn't need to be sent to another host, saved to the db or otherwise used across
// program restarts, a monotonically nondecreasing clock (`Instant`) should be used. But as
// `Instant` may use `libc::clock_gettime(CLOCK_MONOTONIC)`, e.g. on Android, and does not advance
// while being in deep sleep mode, we use `SystemTime` instead, but add an alias for it to document
// why `Instant` isn't used in those places. Also this can help to switch to another clock impl if
// we find any.
pub use std::time::SystemTime as Time;
use std::time::{Duration, SystemTime}; use std::time::{Duration, SystemTime};
use anyhow::{bail, Context as _, Result}; use anyhow::{bail, Context as _, Result};
@@ -482,6 +489,10 @@ pub(crate) fn time() -> i64 {
.as_secs() as i64 .as_secs() as i64
} }
pub(crate) fn time_elapsed(time: &Time) -> Duration {
time.elapsed().unwrap_or_default()
}
/// Struct containing all mailto information /// Struct containing all mailto information
#[derive(Debug, Default, Eq, PartialEq)] #[derive(Debug, Default, Eq, PartialEq)]
pub struct MailTo { pub struct MailTo {