I was digging on why I'm getting so many dropped messages, and with a little research assistance it appears it's all caused by a race condition causing a deadlock.
During startup the syncPasswords is run asynchronously without awaiting, then isInClique is run asynchronously. If the keystore is empty (i.e. you just logged into iCloud or you have nothing in your Apple Wallet, Apple Password Manager, and aren't syncing any WiFi credentials), syncPassword ends up grabbing the read lock on the keystore mutex twice at different times without releasing either. In the meantime however, isInClique needs to grab a write lock on the keystore. There's a high probability race condition that isInClique will queue this write lock after the first syncPasswords read lock and before the second one. This causes a deadlock because the syncPassword has to release the first read lock before isInClique can grab and then release the write lock, but syncPassword is stuck waiting forever on trying to grab the second read lock.
This deadlock during startup blocks the initFuture that's the startup initialization boundary, which causes the Dart processsing of messages to block waiting on startup init to complete. As a result the Rust code trying to hand Dart messages times out waiting on the messages to get handled (30 sec timeout, 5x retry) and drops them.
I happened to have some extra tokens, so I ran a wide-context deep analysis of the mutex usage in the repo. It identified two issues with how the mutexes are currently used:
- Locks on the same data requested at different times but held simultaneously (the type this bug falls into)
- Out of order locking of a sequence of mutexes
Tokio is using queued locking, so it is never safe to hold a lock on some data and then later try to request another lock on it while still holding the original lock. When the second lock is queued it might come after some asynchronous write lock that's already waiting on the first lock to release, which gives you a deadlock.
If you have multiple mutex locks taken by the same execution thread, you must always take them in the same order everywhere. This applies transitively as well, which can be especially hard to trace.
In the simple case of 2 mutexes, A and B, if thread 1 takes A then B, but thread 2 takes B then A, at some point you can run into the situation where thread 1 and thread 2 run at the same time and thread 1 takes A , thread 2 takes B, then thread 1 tries to take B and thread 2 tries to take A. In the transitive case, you might have thread 1 takes A then B, thread 2 takes B then C, and thread 3 takes C then A . At some topint thread 1 holds A, thead 2 holds B, and thread 3 holds C, but thread 1 is waiting on B, thread 2 is waiting on C, and thread 3 is waiting on A. For this transitive case you have to define a codebase-wide ordering within all supersets of mutexes that get used together, and ensure that all code always takes the mutexes they need in that defined order.
I generated a markdown report for this (attached below if you want to see the full details). There are:
- 4 locking twice in the same thread while still holding the original lock (including this Issue)
- 1 out-of-order multi-mutex locking sequence
MUTEX_LOCK_AUDIT.md
I was digging on why I'm getting so many dropped messages, and with a little research assistance it appears it's all caused by a race condition causing a deadlock.
During startup the
syncPasswordsis run asynchronously without awaiting, thenisInCliqueis run asynchronously. If the keystore is empty (i.e. you just logged into iCloud or you have nothing in your Apple Wallet, Apple Password Manager, and aren't syncing any WiFi credentials),syncPasswordends up grabbing the read lock on the keystore mutex twice at different times without releasing either. In the meantime however,isInCliqueneeds to grab a write lock on the keystore. There's a high probability race condition thatisInCliquewill queue this write lock after the firstsyncPasswordsread lock and before the second one. This causes a deadlock because the syncPassword has to release the first read lock beforeisInCliquecan grab and then release the write lock, butsyncPasswordis stuck waiting forever on trying to grab the second read lock.This deadlock during startup blocks the
initFuturethat's the startup initialization boundary, which causes the Dart processsing of messages to block waiting on startup init to complete. As a result the Rust code trying to hand Dart messages times out waiting on the messages to get handled (30 sec timeout, 5x retry) and drops them.I happened to have some extra tokens, so I ran a wide-context deep analysis of the mutex usage in the repo. It identified two issues with how the mutexes are currently used:
Tokio is using queued locking, so it is never safe to hold a lock on some data and then later try to request another lock on it while still holding the original lock. When the second lock is queued it might come after some asynchronous write lock that's already waiting on the first lock to release, which gives you a deadlock.
If you have multiple mutex locks taken by the same execution thread, you must always take them in the same order everywhere. This applies transitively as well, which can be especially hard to trace.
In the simple case of 2 mutexes, A and B, if thread 1 takes A then B, but thread 2 takes B then A, at some point you can run into the situation where thread 1 and thread 2 run at the same time and thread 1 takes A , thread 2 takes B, then thread 1 tries to take B and thread 2 tries to take A. In the transitive case, you might have thread 1 takes A then B, thread 2 takes B then C, and thread 3 takes C then A . At some topint thread 1 holds A, thead 2 holds B, and thread 3 holds C, but thread 1 is waiting on B, thread 2 is waiting on C, and thread 3 is waiting on A. For this transitive case you have to define a codebase-wide ordering within all supersets of mutexes that get used together, and ensure that all code always takes the mutexes they need in that defined order.
I generated a markdown report for this (attached below if you want to see the full details). There are:
MUTEX_LOCK_AUDIT.md