[might revert] Don't attempt multiple connections at once for now

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<String>,
     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::<String>();
-    let relays_from_qr: BTreeSet<String> = 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<Mutex<String>> = 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, &param, 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, &param, 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<bool> {
     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);
     };

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;
 

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(&param.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);

src/context.rs

@@ -232,7 +232,7 @@ pub struct InnerContext {
     /// clients.
     running_state: RwLock<RunningState>,
     /// 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,

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(());
     };