Skip to content

Commit

Permalink
fix(common): Use a trick to avoid hitting the recursion_limit too q…
Browse files Browse the repository at this point in the history
…uickly.

This patch adds a trick around `SyncTimelineEvent` to avoid reaching the
`recursion_limit` too quickly. Read the documentation in this patch to
learn more.
  • Loading branch information
Hywan committed Dec 20, 2024
1 parent 38e35b9 commit 054f5e2
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 3 deletions.
5 changes: 4 additions & 1 deletion crates/matrix-sdk-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ uniffi = ["dep:uniffi", "matrix-sdk-crypto?/uniffi", "matrix-sdk-common/uniffi"]
# Private feature, see
# https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823 for the gory
# details.
test-send-sync = ["matrix-sdk-crypto?/test-send-sync"]
test-send-sync = [
"matrix-sdk-common/test-send-sync",
"matrix-sdk-crypto?/test-send-sync",
]

# "message-ids" feature doesn't do anything and is deprecated.
message-ids = []
Expand Down
4 changes: 4 additions & 0 deletions crates/matrix-sdk-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ targets = ["x86_64-unknown-linux-gnu", "wasm32-unknown-unknown"]
[features]
js = ["wasm-bindgen-futures"]
uniffi = ["dep:uniffi"]
# Private feature, see
# https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823 for the gory
# details.
test-send-sync = []

[dependencies]
async-trait = { workspace = true }
Expand Down
33 changes: 33 additions & 0 deletions crates/matrix-sdk-common/src/deserialized_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,22 @@ pub struct EncryptionInfo {
/// Previously, this differed from [`TimelineEvent`] by wrapping an
/// [`AnySyncTimelineEvent`] instead of an [`AnyTimelineEvent`], but nowadays
/// they are essentially identical, and one of them should probably be removed.
//
// 🚨 Note about this type, please read! 🚨
//
// `SyncTimelineEvent` is heavily used across the SDK crates. In some cases, we
// are reaching a [`recursion_limit`] when the compiler is trying to figure out
// if `SyncTimelineEvent` implements `Sync` when it's embedded in other types.
//
// We want to help the compiler so that one doesn't need to increase the
// `recursion_limit`. We stop the recursive check by (un)safely implement `Sync`
// and `Send` on `SyncTimelineEvent` directly.
//
// See
// https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823
// which has addressed this issue first
//
// [`recursion_limit`]: https://doc.rust-lang.org/reference/attributes/limits.html#the-recursion_limit-attribute
#[derive(Clone, Debug, Serialize)]
pub struct SyncTimelineEvent {
/// The event itself, together with any information on decryption.
Expand All @@ -318,6 +334,23 @@ pub struct SyncTimelineEvent {
pub push_actions: Vec<Action>,
}

// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
#[cfg(not(feature = "test-send-sync"))]
unsafe impl Send for SyncTimelineEvent {}

// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
#[cfg(not(feature = "test-send-sync"))]
unsafe impl Sync for SyncTimelineEvent {}

#[cfg(feature = "test-send-sync")]
#[test]
// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
fn test_send_sync_for_sync_timeline_event() {
fn assert_send_sync<T: Send + Sync>() {}

assert_send_sync::<SyncTimelineEvent>();
}

impl SyncTimelineEvent {
/// Create a new `SyncTimelineEvent` from the given raw event.
///
Expand Down
2 changes: 0 additions & 2 deletions crates/matrix-sdk-ui/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#![recursion_limit = "256"]

use itertools::Itertools as _;
use matrix_sdk::deserialized_responses::TimelineEvent;
use ruma::{events::AnyStateEvent, serde::Raw, EventId, RoomId};
Expand Down

0 comments on commit 054f5e2

Please sign in to comment.