From a8e1110bcda8c7cf69cb66ce8455502b5f85cab5 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Thu, 3 Sep 2026 00:24:00 +0000 Subject: [PATCH] fix: always emit AccountsBackgroundFetchDone A second background_fetch() while one is already running returned without emitting the event, and the FFI returned 1 for it, so a UI waiting for an event hangs dc_get_next_event(). Emit the event in any case, so waiting for it is safe. --- deltachat-ffi/deltachat.h | 19 +++++++++----- deltachat-ffi/src/lib.rs | 7 ++++- deltachat-jsonrpc/src/api.rs | 3 ++- deltachat-jsonrpc/src/api/types/events.rs | 12 ++++++--- src/accounts.rs | 32 ++++++++++++++++++++++- src/events/payload.rs | 12 ++++++--- 6 files changed, 67 insertions(+), 18 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 80beedfb8..844e09d37 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -3193,15 +3193,17 @@ void dc_accounts_maybe_network_lost (dc_accounts_t* accounts); * * dc_accounts_background_fetch() was created for the iOS Background fetch. * - * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end - * even in case of timeout, unless the function fails and returns 0. + * The `DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE` event is emitted at the end, + * also on timeout, when another background fetch is already running + * and when the call is ignored because the timeout is too small, + * so it is safe to wait for the event whenever `accounts` is not NULL. * Process all events until you get this one and you can safely return to the background * without forgetting to create notifications caused by timing race conditions. * * @memberof dc_accounts_t * @param accounts The account manager as created by dc_accounts_new(). * @param timeout The timeout in seconds - * @return Return 1 if DC_EVENT_ACCOUNTS_BACKGROUND_FETCH_DONE was emitted and 0 otherwise. + * @return Return 0 if the call was ignored because `accounts` is NULL or the timeout is too small, 1 otherwise. */ int dc_accounts_background_fetch (dc_accounts_t* accounts, uint64_t timeout); @@ -6364,11 +6366,14 @@ void dc_event_unref(dc_event_t* event); #define DC_EVENT_WEBXDC_REALTIME_ADVERTISEMENT 2151 /** - * Tells that the Background fetch was completed (or timed out). + * Tells that a call to dc_accounts_background_fetch() is done: + * the fetch completed, timed out, was stopped or was not started. + * + * For the call that started the fetch, this event acts as a marker: + * when you reach it, all events emitted during the fetch were processed. + * A call made while another background fetch is running gets the event immediately, + * and the running fetch keeps emitting events until its own marker. * - * This event acts as a marker, when you reach this event you can be sure - * that all events emitted during the background fetch were processed. - * * This event is only emitted by the account manager */ diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 96bd67448..19ab248a6 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4750,12 +4750,17 @@ pub unsafe extern "C" fn dc_accounts_background_fetch( accounts: *const dc_accounts_t, timeout_in_seconds: u64, ) -> libc::c_int { - if accounts.is_null() || timeout_in_seconds <= 2 { + if accounts.is_null() { eprintln!("ignoring careless call to dc_accounts_background_fetch()"); return 0; } let accounts = unsafe { &*accounts }; + if timeout_in_seconds <= 2 { + eprintln!("ignoring careless call to dc_accounts_background_fetch(): timeout too small"); + block_on(accounts.read()).emit_event(EventType::AccountsBackgroundFetchDone); + return 0; + } let background_fetch_future = { let lock = block_on(accounts.read()); lock.background_fetch(Duration::from_secs(timeout_in_seconds)) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index e571ff979..6c66fafd5 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -278,7 +278,8 @@ impl CommandApi { /// Performs a background fetch for all accounts in parallel with a timeout. /// - /// The `AccountsBackgroundFetchDone` event is emitted at the end even in case of timeout. + /// The `AccountsBackgroundFetchDone` event is emitted at the end even in case of timeout, + /// and immediately if another background fetch is already running. /// Process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. async fn background_fetch(&self, timeout_in_seconds: f64) -> Result<()> { diff --git a/deltachat-jsonrpc/src/api/types/events.rs b/deltachat-jsonrpc/src/api/types/events.rs index de193db2b..0bfa3138a 100644 --- a/deltachat-jsonrpc/src/api/types/events.rs +++ b/deltachat-jsonrpc/src/api/types/events.rs @@ -394,11 +394,15 @@ pub enum EventType { msg_id: u32, }, - /// Tells that the Background fetch was completed (or timed out). - /// This event acts as a marker, when you reach this event you can be sure - /// that all events emitted during the background fetch were processed. + /// Tells that a background fetch call is done: + /// the fetch completed, timed out, was stopped or was not started. /// - /// This event is only emitted by the account manager + /// For the call that started the fetch, this event acts as a marker: + /// all events emitted during the fetch were processed once it is reached. + /// A call made while another background fetch is running gets the event immediately, + /// and the running fetch keeps emitting events until its own marker. + /// + /// This event is only emitted by the account manager. AccountsBackgroundFetchDone, /// Inform that set of chats or the order of the chats in the chatlist has changed. /// diff --git a/src/accounts.rs b/src/accounts.rs index 8e5fec8b0..20e901cc8 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -443,7 +443,12 @@ impl Accounts { interrupt_receiver: Option>, ) { let Some(interrupt_receiver) = interrupt_receiver else { - // Nothing to do if we got no interrupt receiver. + // Another background fetch is already running. + // Emit the event anyway so that a caller waiting for it does not hang. + events.emit(Event { + id: 0, + typ: EventType::AccountsBackgroundFetchDone, + }); return; }; if let Err(_err) = tokio::time::timeout( @@ -480,6 +485,8 @@ impl Accounts { /// The `AccountsBackgroundFetchDone` event is emitted at the end, /// process all events until you get this one and you can safely return to the background /// without forgetting to create notifications caused by timing race conditions. + /// If another background fetch is already running, + /// nothing is fetched and the event is emitted immediately. /// /// Returns a future that resolves when background fetch is done, /// but does not capture `&self`. @@ -1239,6 +1246,29 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_background_fetch_emits_done_when_already_running() -> Result<()> { + let dir = tempfile::tempdir()?; + let writable = true; + let accounts = Accounts::new(dir.path().join("accounts"), writable).await?; + let event_emitter = accounts.get_event_emitter(); + + let timeout = std::time::Duration::from_secs(3); + let first = accounts.background_fetch(timeout); + let second = accounts.background_fetch(timeout); + tokio::join!(first, second); + + let mut done = 0; + while let Ok(event) = event_emitter.try_recv() { + if matches!(event.typ, EventType::AccountsBackgroundFetchDone) { + done += 1; + } + } + assert_eq!(done, 2); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_encrypted_account() -> Result<()> { let dir = tempfile::tempdir().context("failed to create tempdir")?; diff --git a/src/events/payload.rs b/src/events/payload.rs index 4e07d6709..383714a77 100644 --- a/src/events/payload.rs +++ b/src/events/payload.rs @@ -357,11 +357,15 @@ pub enum EventType { msg_id: MsgId, }, - /// Tells that the Background fetch was completed (or timed out). - /// This event acts as a marker, when you reach this event you can be sure - /// that all events emitted during the background fetch were processed. + /// Tells that a background fetch call is done: + /// the fetch completed, timed out, was stopped or was not started. /// - /// This event is only emitted by the account manager + /// For the call that started the fetch, this event acts as a marker: + /// all events emitted during the fetch were processed once it is reached. + /// A call made while another background fetch is running gets the event immediately, + /// and the running fetch keeps emitting events until its own marker. + /// + /// This event is only emitted by the account manager. AccountsBackgroundFetchDone, /// Inform that set of chats or the order of the chats in the chatlist has changed. ///