mirror of
https://github.com/chatmail/core.git
synced 2026-04-17 21:46:35 +03:00
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 <https://docs.rs/tokio/1.44.1/tokio/sync/struct.RwLock.html#method.read>: "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."
This commit is contained in:
@@ -227,8 +227,9 @@ impl CommandApi {
|
||||
/// Get a list of all configured accounts.
|
||||
async fn get_all_accounts(&self) -> Result<Vec<Account>> {
|
||||
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?)
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user