matrix-org / matrix-org/matrix-rust-sdk
Move `ThreadSummary` from `TimelineEvent` to `ThreadInfo`
- Dominant language
- Rust
- Stars
- 2.3k
- Forks
- 500
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 106
Description
`TimelineEvent` holds a `ThreadSummary` field.
The `TimelineEvent` struct:
https://github.com/matrix-org/matrix-rust-sdk/blob/d133dcb28a680ecbfee43266393e4086ea49e0c7/crates/matrix-sdk-common/src/deserialized_responses.rs#L492-L531
The `ThreadSummary` enum:
https://github.com/matrix-org/matrix-rust-sdk/blob/d133dcb28a680ecbfee43266393e4086ea49e0c7/crates/matrix-sdk-common/src/deserialized_responses.rs#L434-L443
What does it mean? It means that _all_ events have this `thread_summary: ThreadSummaryStatus` field. Sadly, this is not true every time. Every event from an unthreaded timeline can be a thread root. However, it's impossible for an event from a thread timeline to be a thread root, [this is forbidden by the specification](https://spec.matrix.org/v1.19/client-server-api/#threading):
> Threads are established using a `rel_type` of `m.thread` and reference the _thread root_ (the main timeline event to which the thread events refer). It is not possible to create a thread from an event which itself is the child of an event relationship (i.e., one with an `m.relates_to` property with a `rel_type` property - see [Relationship types](https://spec.matrix.org/v1.19/client-server-api/#relationship-types)). It is therefore also not possible to nest threads.
We are updating the `ThreadSummary` for events in the Event Cache for a room cache (`RoomEventCache`). However, we don't do that for other caches, like thread cache (`ThreadEventCache`) or event-focused cache (`EventFocusedCache`) because _theoretically_, the thread root **IS NOT** part of the threaded timeline as the specification says. It means that the `ThreadSummary` is only set for unthreaded events, so only events owned by the room cache.
But (there is always a but), because clients need to display the thread root within a thread, for the sake of commodity, the thread root is added to the thread cache on pagination:
https://github.com/matrix-org/matrix-rust-sdk/blob/d133dcb28a680ecbfee43266393e4086ea49e0c7/crates/matrix-sdk/src/event_cache/caches/thread/pagination.rs#L292-L307
And now we have a “disconnection”. The in-memory `ThreadSummary` value of the thread root event in a `RoomEventCache` can be out-of-sync with the in-memory `ThreadSummary` value of the same event in a `ThreadEventCache` because we only update the `ThreadSummary` of the former cache. This problem has been rightfully raised by @zzorba in https://github.com/matrix-org/matrix-rust-sdk/pull/7015.
The problem does **NOT** exist in the database though, because all events are stored in a unique table.
To solve this problem, we can do a similar approach to https://github.com/matrix-org/matrix-rust-sdk/pull/7015, but the problem can occur in other event caches. To permanently and robustly solve this problem, I propose to extract `ThreadSummary` outside `TimelineEvent` and move it inside the new `ThreadInfo` struct (this type has been recently introduced in https://github.com/matrix-org/matrix-rust-sdk/pull/6893). Of course, it also implies to move the `TimelineEvent::bundled_latest_thread_event` field too, as it relates to the `ThreadSummary`.
> [!NOTE]
>
> Indirectly, the size of `TimelineEvent` will shrink from 160 bytes to 120 bytes, which is not negligible!
With this approach, we will have **a unique place** where the `ThreadSummary` would live.
We will also remove a possible delicate inter-locking situation between 2 caches, or R2D2 missing to update the `ThreadSummary`. Indeed, the `ThreadSummary` is computed by the `ThreadEventCache` or R2D2, but is stored in the `RoomEventCache`:
https://github.com/matrix-org/matrix-rust-sdk/blob/d133dcb28a680ecbfee43266393e4086ea49e0c7/crates/matrix-sdk/src/event_cache/caches/mod.rs#L368-L373
And this has been the source of a deadlock or bugs in the past, see:
- https://github.com/matrix-org/matrix-rust-sdk/commit/f20ec6edaf5946c39875669b8044d1ada8d6f94f
- https://github.com/matrix-org/matrix-rust-sdk/pull/6619
---
- Somehow related to https://github.com/matrix-org/matrix-rust-sdk/issues/6013
Contributor guide
Assessment
This issue has not been assessed yet.