From 83b870b32844fe4cd6b4a6c322cde616d47c41c2 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Fri, 18 Sep 2026 18:24:00 +0200 Subject: [PATCH] [might revert] Don't attempt multiple connections at once for now --- src/autorelay.rs | 121 +++++++------------------------ src/autorelay/autorelay_tests.rs | 35 +-------- src/configure.rs | 9 +-- src/context.rs | 4 +- src/sql.rs | 2 +- 5 files changed, 35 insertions(+), 136 deletions(-) diff --git a/src/autorelay.rs b/src/autorelay.rs index bf411cfea..f3f17b6e2 100644 --- a/src/autorelay.rs +++ b/src/autorelay.rs @@ -6,27 +6,19 @@ //! which migrations seed with a list of known chatmail relays. //! //! Status of implementation: -//! -//! - When the UI uses `init_transports()`, we attempt to add 3 relays. -//! -//! - Later additions are attempted right before going into IMAP IDLE, -//! i.e. only while connected and with nothing more important to do, -//! and only if a UI opted in via [`Config::Autorelay`]. -//! Once a profile has reached `NUM_TRANSPORTS_TARGET` transports, -//! [`Config::AutorelayFinished`] is set and nothing is ever added again, -//! so deleting a transport later does not pull in a replacement. +//! Additions are attempted right before going into IMAP IDLE, +//! i.e. only while connected and with nothing more important to do, +//! and only if a UI opted in via [`Config::Autorelay`]. +//! Once a profile has reached `NUM_TRANSPORTS_TARGET` transports, +//! [`Config::AutorelayFinished`] is set and nothing is ever added again, +//! so deleting a transport later does not pull in a replacement. -use std::collections::BTreeSet; use std::pin::Pin; -use std::sync::Arc; -use anyhow::{Result, bail}; -use deltachat_contact_tools::{EmailAddress, addr_normalize}; +use anyhow::Result; +use deltachat_contact_tools::addr_normalize; use rand::distr::{Alphanumeric, SampleString}; -use rand::rng; -use rand::seq::{IndexedRandom, SliceRandom as _}; -use tokio::sync::Mutex; -use tokio::task::JoinSet; +use rand::seq::IndexedRandom; use crate::config::{self, Config}; use crate::log::{LogExt, warn}; @@ -64,85 +56,26 @@ pub(crate) async fn init_transports_inner( addrs_from_qr: Vec, skip_network: bool, ) -> Result<()> { - // If relays were provided via the addresses in the QR code, - // then these relays are tried first. - let (relays_sender, relays_receiver) = async_channel::unbounded::(); - let relays_from_qr: BTreeSet = addrs_from_qr - .into_iter() - .filter_map(|addr| EmailAddress::new(&addr).ok()) - .map(|email| email.domain) - .collect(); - for relay in &relays_from_qr { - relays_sender.try_send(relay.to_string())?; - } - - // After the relays from the QR code, - // the default relays are tried in a random order. - let mut default_relays: Vec<&str> = DEFAULT_RELAY_CANDIDATES.into(); - default_relays.shuffle(&mut rng()); - for relay in default_relays { - if !relays_from_qr.contains(relay) { - relays_sender.try_send(relay.to_string())?; - } - } - - let last_error: Arc> = Default::default(); - - // Spawn NUM_TRANSPORTS_TARGET tasks that each add a relay concurrently. - let mut join_set = JoinSet::new(); - for _ in 0..NUM_TRANSPORTS_TARGET { - let context = context.clone(); - let relays_receiver = relays_receiver.clone(); - let last_error = last_error.clone(); - join_set.spawn(async move { - // Take a lock in order to prevent other relay management code - // from running simultaneously - let _lock = context.background_task_lock.read().await; - // TODO add a back-channel here, - // and then the surrounding function has to wait until all tasks took the lock - - loop { - let Ok(host) = relays_receiver.try_recv() else { - return false; // No more relays to try - }; - let param = login_param_from_host(&host); - let res = crate::configure::configure(&context, ¶m, skip_network).await; - if let Err(err) = res { - warn!(context, "Failed to init transport {host}: {err:#}."); - *last_error.lock().await = format!("{err:#}"); - // Try another relay in the next iteration of the loop - } else { - info!(context, "Initialized with transport {host}"); - if context.count_transports().await.unwrap_or(0) >= NUM_TRANSPORTS_TARGET { - context - .set_config_bool(Config::AutorelayFinished, true) - .await - .log_err(&context) - .ok(); - info!(context, "Target number of transports reached."); - context.restart_io_if_running().await; - } - return true; // Success - } + context + .sql + .transaction(|transaction| { + let mut stmt = transaction.prepare("INSERT INTO relay_candidates(host) VALUES(?)")?; + for addr in addrs_from_qr { + stmt.execute((addr,))?; } - }); - } + Ok(()) + }) + .await?; - loop { - match join_set.join_next().await { - Some(Ok(true)) => break, // Success - Some(Ok(false)) => {} // Wait until one of the other tasks is successful - Some(Err(e)) => warn!(context, "One of the init_transports tasks failed: {e:#}"), - None => bail!( - "Could not configure any relay, are you offline? ({})", - last_error.try_lock()? - ), - } + let host = "nine.testrun.org"; + let param = login_param_from_host(host); + let res = crate::configure::configure(context, ¶m, skip_network).await; + if let Err(err) = &res { + warn!(context, "Failed to init transports: {err:#}."); + } else { + info!(context, "Initialized with transport {host}"); } - - // Let the other tasks continue running in the background - // while the user can already use Delta Chat: - join_set.detach_all(); + res?; context.set_config_bool(Config::Autorelay, true).await?; @@ -174,7 +107,7 @@ pub(crate) fn maybe_add_additional_relays( async fn maybe_add_additional_relays_inner(context: &Context, skip_network: bool) -> Result { let now = time(); - let Ok(_lock) = context.background_task_lock.try_write() else { + let Ok(_lock) = context.background_task_mutex.try_lock() else { // Housekeeping or automatic relay management is already running in another thread, do nothing. return Ok(false); }; diff --git a/src/autorelay/autorelay_tests.rs b/src/autorelay/autorelay_tests.rs index 1e2f359c8..add3c0652 100644 --- a/src/autorelay/autorelay_tests.rs +++ b/src/autorelay/autorelay_tests.rs @@ -12,44 +12,13 @@ async fn test_init_transports_basic() -> Result<()> { let skip_network = true; init_transports_inner(t, vec![], skip_network).await?; - // Wait until the tasks adding transports are finished: - let _ = t.background_task_lock.write().await; - let relays = get_configured_relays(t).await; - assert_eq!(relays.len(), NUM_TRANSPORTS_TARGET); + assert_eq!(relays.len(), 1); for relay in &relays { assert!(DEFAULT_RELAY_CANDIDATES.contains(&relay.as_ref())); } assert_eq!(t.get_config_bool(Config::Autorelay).await?, true); - assert_eq!(t.get_config_bool(Config::AutorelayFinished).await?, true); - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_init_transports_use_relays_from_qr() -> Result<()> { - let t = &TestContext::new().await; - - let skip_network = true; - init_transports_inner( - t, - vec![ - "alice@example.org".to_string(), - "bob@example.org".to_string(), - "bob@nine.testrun.org".to_string(), - ], - skip_network, - ) - .await?; - - // Wait until the tasks adding transports are finished: - let _ = t.background_task_lock.write().await; - - let relays = get_configured_relays(t).await; - assert_eq!(relays.len(), NUM_TRANSPORTS_TARGET); - assert!(relays.contains(&"example.org".to_string())); - assert!(relays.contains(&"nine.testrun.org".to_string())); Ok(()) } @@ -165,7 +134,7 @@ async fn test_maybe_add_additional_relays_mutex_held() -> Result<()> { // Hold the housekeeping mutex ourselves, simulating another task // already running housekeeping or relay management. - let _lock = t.background_task_lock.write().await; + let _lock = t.background_task_mutex.lock().await; assert_autorelay_does_nothing(t).await; diff --git a/src/configure.rs b/src/configure.rs index e8102f22f..832c45c09 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -232,9 +232,6 @@ impl Context { bail!(error_msg); } - self.update_device_chats() - .await - .context("Failed to update device chats")?; progress!(self, 1000); self.start_io().await; @@ -332,9 +329,6 @@ impl Context { ); return Err(error); }; - self.update_device_chats() - .await - .context("Failed to update device chats")?; if provider::legacy_settings_for_addr(¶m.addr)?.worse_media_quality && !self.config_exists(Config::MediaQuality).await? { @@ -578,6 +572,9 @@ pub(crate) async fn configure( ctx.scheduler.interrupt_inbox().await; progress!(ctx, 940); + ctx.update_device_chats() + .await + .context("Failed to update device chats")?; ctx.sql.set_raw_config_bool("configured", true).await?; ctx.emit_event(EventType::AccountsItemChanged); diff --git a/src/context.rs b/src/context.rs index d062611b3..8673a7be0 100644 --- a/src/context.rs +++ b/src/context.rs @@ -232,7 +232,7 @@ pub struct InnerContext { /// clients. running_state: RwLock, /// Lock to prevent running housekeeping or relay management from multiple threads at once. - pub(crate) background_task_lock: RwLock<()>, + pub(crate) background_task_mutex: Mutex<()>, /// Mutex to prevent multiple IMAP loops from fetching the messages at once. /// @@ -487,7 +487,7 @@ impl Context { blobdir, running_state: RwLock::new(Default::default()), sql: Sql::new(dbfile), - background_task_lock: RwLock::new(()), + background_task_mutex: Mutex::new(()), fetch_msgs_mutex: Mutex::new(()), translated_stockstrings: stockstrings, events, diff --git a/src/sql.rs b/src/sql.rs index d13f72810..b79952b27 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -814,7 +814,7 @@ async fn incremental_vacuum(context: &Context) -> Result<()> { /// Cleanup the account to restore some storage and optimize the database. pub async fn housekeeping(context: &Context) -> Result<()> { - let Ok(_housekeeping_lock) = context.background_task_lock.try_write() else { + let Ok(_housekeeping_lock) = context.background_task_mutex.try_lock() else { // Housekeeping is already running in another thread, do nothing. return Ok(()); };