From 70563867a607191f466704113aa1b9a246a23a8f Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 1 Apr 2025 11:25:30 +0000 Subject: [PATCH] fix(jsonrpc): fix deadlock in get_all_accounts() `self.accounts.read().await.get_all()` acquires a read lock and does not release it until the end of `for` loop. After that, a writer may get into the queue, e.g. because of the concurrent `add_account` call. In this case `let context_option = self.accounts.read().await.get_account(id);` tries to acquire another read lock and deadlocks because tokio RwLock is write-preferring and will not give another read lock while there is a writer in the queue. At the same time, writer never gets a write lock because the first read lock is not released. The fix is to get a single read lock for the whole `get_all_accounts()` call. This is described in : "Note that under the priority policy of RwLock, read locks are not granted until prior write locks, to prevent starvation. Therefore deadlock may occur if a read lock is held by the current task, a write lock attempt is made, and then a subsequent read lock attempt is made by the current task." --- deltachat-jsonrpc/src/api.rs | 5 +++-- deltachat-rpc-client/tests/test_something.py | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index 50aecfd23..2ab1f4fad 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -227,8 +227,9 @@ impl CommandApi { /// Get a list of all configured accounts. async fn get_all_accounts(&self) -> Result> { let mut accounts = Vec::new(); - for id in self.accounts.read().await.get_all() { - let context_option = self.accounts.read().await.get_account(id); + let accounts_lock = self.accounts.read().await; + for id in accounts_lock.get_all() { + let context_option = accounts_lock.get_account(id); if let Some(ctx) = context_option { accounts.push(Account::from_context(&ctx, id).await?) } diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index db59ac23f..4ba5fc7be 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -797,3 +797,11 @@ def test_rename_group(acfactory): alice_group.set_name(name) bob.wait_for_incoming_msg_event() assert bob_chat.get_basic_snapshot().name == name + + +def test_get_all_accounts_deadlock(rpc): + """Regression test for get_all_accounts deadlock.""" + for _ in range(100): + all_accounts = rpc.get_all_accounts.future() + rpc.add_account() + all_accounts()