fix(sdk): Remove a cyclic reference of Client in ThreadSubscriptionCatchup

[Hywan]

This patch fixes cyclic reference of Client in ThreadSubscriptionCatchup.

ThreadSubscriptionCatchup starts a task that call thread_subscriptions_catchup_task. This function captures a clone of ThreadSubscriptionCatchup (why not…) which contains a WeakClient, all good, no cycle here! However, the real task (not the function) captures Client to call enabled_thread_subscriptions to know if the function thread_subscriptions_catchup_task must be called. Consequently, the task captures a clone of Client, boom, we have a cycle.

This patch fixes the problem by spawning the task if and only if Client::enabled_thread_subscriptions returns Ok(true). This check is done outside the task. It seems saner and avoid creating this cycle.

Consequently, the ThreadSubscriptionCatchup::new method becomes async, which is perfectly fine as it was already wrapper inside an async block when initialised by the Client.

Without the fix, the test fails with:

thread 'client::thread_subscriptions::timed_tests::test_issue_6573_client_can_drop_thread_subscriptions_task' (2625650) panicked at crates/matrix-sdk/src/client/thread_subscriptions.rs:514:9:
assertion `left == right` failed
  left: 1
 right: 0

as expected.

---

• Fix Arc<ClientInner> reference cycle via ThreadSubscriptionCatchup prevents Client drop in matrix-sdk 0.17.0 #6573

---

• I've documented the public API changes in the appropriate changelog files
  (see Writing changelog entries).
• This PR was made with the help of AI.

[codspeed-hq]

Merging this PR will not alter performance

✅ 50 untouched benchmarks

---

_{Comparing Hywan:fix-issue-6573 (a7cbf88) with main (b847983)}

[codecov]

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.95%. Comparing base (7677f09) to head (a7cbf88).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ates/matrix-sdk/src/client/thread_subscriptions.rs	97.14%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6594      +/-   ##
==========================================
- Coverage   89.96%   89.95%   -0.01%     
==========================================
  Files         382      382              
  Lines      107797   107810      +13     
  Branches   107797   107810      +13     
==========================================
+ Hits        96979    96982       +3     
- Misses       7153     7160       +7     
- Partials     3665     3668       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[poljar]

A bit annoying that we added a test-only field, though it seems to me that we can get rid of it.

[poljar]

Is this needed? Can't you use OnceCell::initialized() on the thread_subscription_catchup field for this?

[Hywan]

We are using OnceLock, not OnceCell, which also has an initialized method, but it's private! I've changed for tsc._task.get().is_some() and it works nicely.

Thanks for noticing this!

Rebasing and merging.