From 83d2e6b8b436163640e273cccfc27d82d6c7f970 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 22 Oct 2023 15:54:31 +0000 Subject: [PATCH] fix: do not interrupt IMAP loop from get_connectivity_html() Android calls get_connectivity_html() every time connectivity changes, which in turn interrupts IMAP loop and triggers change from "not connected" to "connecting" state. To avoid such infinite loop of IMAP interrupts when there is not connectivity, update quota only when IMAP loop is interrupted otherwise. This anyway happens when a message is received or maybe_network is called. Also remove outdated comments about `Action::UpdateRecentQuota` job which does not exist anymore. --- src/context.rs | 4 ---- src/quota.rs | 24 ------------------------ src/scheduler.rs | 18 ++++++++++++++---- src/scheduler/connectivity.rs | 10 +--------- src/sql.rs | 2 -- 5 files changed, 15 insertions(+), 43 deletions(-) diff --git a/src/context.rs b/src/context.rs index 22dc265c7..c1e690632 100644 --- a/src/context.rs +++ b/src/context.rs @@ -211,9 +211,6 @@ pub struct InnerContext { /// Set to `None` if quota was never tried to load. pub(crate) quota: RwLock>, - /// Set to true if quota update is requested. - pub(crate) quota_update_request: AtomicBool, - /// IMAP UID resync request. pub(crate) resync_request: AtomicBool, @@ -384,7 +381,6 @@ impl Context { scheduler: SchedulerState::new(), ratelimit: RwLock::new(Ratelimit::new(Duration::new(60, 0), 6.0)), // Allow at least 1 message every 10 seconds + a burst of 6. quota: RwLock::new(None), - quota_update_request: AtomicBool::new(false), resync_request: AtomicBool::new(false), new_msgs_notify, server_id: RwLock::new(None), diff --git a/src/quota.rs b/src/quota.rs index f2a4b921d..5fc976437 100644 --- a/src/quota.rs +++ b/src/quota.rs @@ -1,7 +1,6 @@ //! # Support for IMAP QUOTA extension. use std::collections::BTreeMap; -use std::sync::atomic::Ordering; use anyhow::{anyhow, Context as _, Result}; use async_imap::types::{Quota, QuotaResource}; @@ -13,7 +12,6 @@ use crate::imap::scan_folders::get_watched_folders; use crate::imap::session::Session as ImapSession; use crate::imap::Imap; use crate::message::{Message, Viewtype}; -use crate::scheduler::InterruptInfo; use crate::tools::time; use crate::{stock_str, EventType}; @@ -34,17 +32,12 @@ pub const QUOTA_ERROR_THRESHOLD_PERCENTAGE: u64 = 95; /// providers report bad values and we would then spam the user. pub const QUOTA_ALLCLEAR_PERCENTAGE: u64 = 75; -/// if recent quota is older, -/// it is re-fetched on dc_get_connectivity_html() -pub const QUOTA_MAX_AGE_SECONDS: i64 = 60; - /// Server quota information with an update timestamp. #[derive(Debug)] pub struct QuotaInfo { /// Recently loaded quota information. /// set to `Err()` if the provider does not support quota or on other errors, /// set to `Ok()` for valid quota information. - /// Updated by `Action::UpdateRecentQuota` pub(crate) recent: Result>>, /// Timestamp when structure was modified. @@ -110,18 +103,6 @@ pub fn needs_quota_warning(curr_percentage: u64, warned_at_percentage: u64) -> b } impl Context { - // Adds a job to update `quota.recent` - pub(crate) async fn schedule_quota_update(&self) -> Result<()> { - let requested = self.quota_update_request.swap(true, Ordering::Relaxed); - if !requested { - // Quota update was not requested before. - self.scheduler - .interrupt_inbox(InterruptInfo::new(false)) - .await; - } - Ok(()) - } - /// Updates `quota.recent`, sets `quota.modified` to the current time /// and emits an event to let the UIs update connectivity view. /// @@ -130,8 +111,6 @@ impl Context { /// As the message is added only once, the user is not spammed /// in case for some providers the quota is always at ~100% /// and new space is allocated as needed. - /// - /// Called in response to `Action::UpdateRecentQuota`. pub(crate) async fn update_recent_quota(&self, imap: &mut Imap) -> Result<()> { if let Err(err) = imap.prepare(self).await { warn!(self, "could not connect: {:#}", err); @@ -166,9 +145,6 @@ impl Context { } } - // Clear the request to update quota. - self.quota_update_request.store(false, Ordering::Relaxed); - *self.quota.write().await = Some(QuotaInfo { recent: quota, modified: time(), diff --git a/src/scheduler.rs b/src/scheduler.rs index ff863910f..c47c1d5a0 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -360,10 +360,20 @@ async fn inbox_loop( info = Default::default(); } None => { - let quota_requested = ctx.quota_update_request.swap(false, Ordering::Relaxed); - if quota_requested { - if let Err(err) = ctx.update_recent_quota(&mut connection).await { - warn!(ctx, "Failed to update quota: {:#}.", err); + { + // Update quota no more than once a minute. + let quota_needs_update = { + let quota = ctx.quota.read().await; + quota + .as_ref() + .filter(|quota| quota.modified + 60 > time()) + .is_none() + }; + + if quota_needs_update { + if let Err(err) = ctx.update_recent_quota(&mut connection).await { + warn!(ctx, "Failed to update quota: {:#}.", err); + } } } diff --git a/src/scheduler/connectivity.rs b/src/scheduler/connectivity.rs index 4d8a66b91..0c72220e4 100644 --- a/src/scheduler/connectivity.rs +++ b/src/scheduler/connectivity.rs @@ -8,10 +8,7 @@ use tokio::sync::Mutex; use crate::events::EventType; use crate::imap::{scan_folders::get_watched_folder_configs, FolderMeaning}; -use crate::quota::{ - QUOTA_ERROR_THRESHOLD_PERCENTAGE, QUOTA_MAX_AGE_SECONDS, QUOTA_WARN_THRESHOLD_PERCENTAGE, -}; -use crate::tools::time; +use crate::quota::{QUOTA_ERROR_THRESHOLD_PERCENTAGE, QUOTA_WARN_THRESHOLD_PERCENTAGE}; use crate::{context::Context, log::LogExt}; use crate::{stock_str, tools}; @@ -472,14 +469,9 @@ impl Context { ret += format!("
  • {e}
    • ").as_str(); } } - - if quota.modified + QUOTA_MAX_AGE_SECONDS < time() { - self.schedule_quota_update().await?; - } } else { let not_connected = stock_str::not_connected(self).await; ret += &format!("
  • {not_connected}
    • "); - self.schedule_quota_update().await?; } ret += ""; diff --git a/src/sql.rs b/src/sql.rs index 521280f62..82199c1d3 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -742,8 +742,6 @@ pub async fn housekeeping(context: &Context) -> Result<()> { warn!(context, "Failed to deduplicate peerstates: {:#}.", err) } - context.schedule_quota_update().await?; - // Try to clear the freelist to free some space on the disk. This // only works if auto_vacuum is enabled. match context