be conservative and take also potentially pending to-device requests into account

Author: niklasb

crates/matrix-sdk-crypto/src/olm/group_sessions/outbound.rs

@@ -160,7 +160,7 @@ pub struct OutboundGroupSession {
     pub(crate) shared_with_set:
         Arc<StdRwLock<BTreeMap<OwnedUserId, BTreeMap<OwnedDeviceId, ShareInfo>>>>,
     #[allow(clippy::type_complexity)]
-    to_share_with_set:
+    pub(crate) to_share_with_set:
         Arc<StdRwLock<BTreeMap<OwnedTransactionId, (Arc<ToDeviceRequest>, ShareInfoSet)>>>,
 }
 

crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs

@@ -427,22 +427,49 @@ fn is_session_overshared_for_user(
     let recipient_device_ids: BTreeSet<&DeviceId> =
         recipient_devices.iter().map(|d| d.device_id()).collect();
 
+    let mut shared: Vec<&DeviceId> = Vec::new();
+
+    // This duplicates a conservative subset of the logic in OutbundGroupSession::is_shared_with,
+    // because we don't have corresponding DeviceData at hand
+    fn is_actually_shared(info: &ShareInfo) -> bool {
+        match info {
+            ShareInfo::Shared(_) => true,
+            ShareInfo::Withheld(_) => false,
+        }
+    }
+
+    // Collect the devices that have definitely received the session already
     let guard = outbound_session.shared_with_set.read();
+    if let Some(for_user) = guard.get(user_id) {
+        shared.extend(for_user.iter().filter_map(|(d, info)| if is_actually_shared(info) {
+            Some(AsRef::<DeviceId>::as_ref(d))
+        } else {
+            None
+        }));
+    }
 
-    let Some(shared) = guard.get(user_id) else {
+    // To be conservative, also collect the devices that would still receive the session
+    // from a pending to-device request if we don't rotate beforehand
+    let guard = outbound_session.to_share_with_set.read();
+    for (_txid, share_infos) in guard.values() {
+        if let Some(for_user) = share_infos.get(user_id) {
+            shared.extend(for_user.iter().filter_map(|(d, info)| if is_actually_shared(info) {
+                Some(AsRef::<DeviceId>::as_ref(d))
+            } else {
+                None
+            }));
+        }
+    }
+
+    if shared.is_empty() {
         return false;
-    };
+    }
 
-    // Devices that received this session
-    let shared: BTreeSet<&DeviceId> = shared
-        .iter()
-        .filter(|(_, info)| matches!(info, ShareInfo::Shared(_)))
-        .map(|(d, _)| d.as_ref())
-        .collect();
+    let shared: BTreeSet<&DeviceId> = shared.into_iter().collect();
 
     // The set difference between
     //
-    // 1. Devices that had previously received the session, and
+    // 1. Devices that had previously received (or are queued to receive) the session, and
     // 2. Devices that would now receive the session
     //
     // Represents newly deleted or blacklisted devices. If this