From 2e980f515ab085f021c2075fe55f6ff417c2ddde Mon Sep 17 00:00:00 2001 From: Salientekill Date: Sun, 3 May 2026 17:50:45 +0200 Subject: [PATCH 1/6] =?UTF-8?q?perf(bot):=20MessageContext.message=20Box?= =?UTF-8?q?=E2=86=92Arc=20+=20from=5Farc=20constructor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloning MessageContext (e.g., re-spawning into a fire-and-forget background task) deep-copied wa::Message every time. wa::Message carries media buffers and extended_text payloads (often >1 KB) — significant overhead on hot paths that fan out per-message work (emoji-challenge dispatch, sticker triggers). Switching to Arc makes Clone an O(1) refcount bump. New from_arc() constructor lets callers that already own an Arc avoid the one remaining clone in from_parts(). BREAKING (minor): the public field type changed from Box to Arc. Read-only access (&self.message, .field, deref) is unaffected. Code that moves out of the field (let msg = *ctx.message) needs (*ctx.message).clone() instead. --- src/bot.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index e73a0d69e..0fae11549 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -30,8 +30,12 @@ pub enum BotBuilderError { Other(#[from] anyhow::Error), } +/// `Arc` so cloning the context to re-spawn into a fire-and-forget +/// task only bumps a refcount; deep-cloning per spawn was costly on hot paths +/// (emoji-challenge, sticker triggers) where the message carries media buffers. +#[derive(Clone)] pub struct MessageContext { - pub message: Box, + pub message: Arc, pub info: MessageInfo, pub client: Arc, } @@ -39,7 +43,16 @@ pub struct MessageContext { impl MessageContext { pub fn from_parts(message: &wa::Message, info: &MessageInfo, client: Arc) -> Self { Self { - message: Box::new(message.clone()), + message: Arc::new(message.clone()), + info: info.clone(), + client, + } + } + + /// Zero-clone alternative when the caller already owns an `Arc`. + pub fn from_arc(message: Arc, info: &MessageInfo, client: Arc) -> Self { + Self { + message, info: info.clone(), client, } From 1230131c282bd048eaa0e784be9b6df48816a601 Mon Sep 17 00:00:00 2001 From: Salientekill Date: Sun, 3 May 2026 21:59:41 +0200 Subject: [PATCH 2/6] perf(bot): MessageContext field private + accessors (CodeRabbit feedback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit's review on PR #613: making the public field type change Box → Arc a controlled breaking change instead of a silent ABI break. Changes: - 'message' field is now private; readers go through new accessors - pub fn message(&self) -> &wa::Message — replaces direct field access - pub fn message_arc(&self) -> Arc — cheap Arc::clone for re-spawn / shared ownership without exposing the inner storage - pub fn into_box(self) -> Box — compatibility helper for legacy code; uses Arc::try_unwrap when the Arc is uniquely held, falling back to deep clone otherwise Read-only access pattern stays nearly identical (ctx.message.field → ctx.message().field). Re-spawns get a typed API for cheap sharing. Old code that needed Box can call into_box() during migration. Storage representation is now an implementation detail behind getters. --- src/bot.rs | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index 0fae11549..3317cf0fd 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -30,12 +30,18 @@ pub enum BotBuilderError { Other(#[from] anyhow::Error), } -/// `Arc` so cloning the context to re-spawn into a fire-and-forget -/// task only bumps a refcount; deep-cloning per spawn was costly on hot paths -/// (emoji-challenge, sticker triggers) where the message carries media buffers. +/// `message` lives behind `Arc` so cloning the context (e.g., re-spawning into +/// a fire-and-forget task) only bumps a refcount — deep-cloning per spawn was +/// costly on hot paths (emoji-challenge, sticker triggers) where the message +/// carries media buffers. +/// +/// The field is private to keep the storage representation an implementation +/// detail. Use [`Self::message`] for read-only access (95% of call sites), +/// [`Self::message_arc`] when you need to share ownership (e.g., re-spawn), +/// or [`Self::into_box`] for legacy code that requires `Box`. #[derive(Clone)] pub struct MessageContext { - pub message: Arc, + message: Arc, pub info: MessageInfo, pub client: Arc, } @@ -58,6 +64,32 @@ impl MessageContext { } } + /// Read-only access to the underlying message. Replaces direct + /// `ctx.message` field access from the previous `Box`-based API. + #[inline] + pub fn message(&self) -> &wa::Message { + &self.message + } + + /// Cheap clone of the inner `Arc` for sharing ownership across tasks + /// without deep-copying `wa::Message` (typical for [`Self::from_arc`] + /// re-spawns). Calling `Arc::clone` directly via `&ctx.message` is no + /// longer available since the field is private. + #[inline] + pub fn message_arc(&self) -> Arc { + Arc::clone(&self.message) + } + + /// Compatibility helper for legacy code that needs `Box` + /// ownership. Tries to reclaim the inner `Message` when the `Arc` is + /// uniquely held; falls back to deep-cloning otherwise. + pub fn into_box(self) -> Box { + match Arc::try_unwrap(self.message) { + Ok(msg) => Box::new(msg), + Err(arc) => Box::new((*arc).clone()), + } + } + pub fn from_event(event: &Event, client: Arc) -> Option { let (msg, info) = event.as_message()?; Some(Self::from_parts(msg, info, client)) From af031187e94ffb86d43a38579258fe881edc3e8c Mon Sep 17 00:00:00 2001 From: Salientekill Date: Sun, 3 May 2026 22:16:56 +0200 Subject: [PATCH 3/6] test(bot): cobrir Arc compatibility boundary do MessageContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endereça segundo review do CodeRabbit no PR #613 — pediu testes focados nos métodos novos. 3 testes novos: - from_arc_preserves_allocation: from_arc não deep-clona; message_arc() retorna o mesmo Arc allocation (Arc::ptr_eq via Arc::as_ptr). - into_box_reclaims_when_unique: refcount==1 → try_unwrap path consome o Arc; weak sentinel fica órfão (strong_count=0, upgrade=None). - into_box_clones_when_shared: refcount>1 → fallback clone preserva conteúdo + kept Arc retorna a strong_count=1, alocação distinta da do Box (verificado via std::ptr::eq negativo). Pointer identity comentada onde não funciona (Box::new realloca o T mesmo após Arc::try_unwrap por mover o T pra novo heap slot). --- src/bot.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/bot.rs b/src/bot.rs index 3317cf0fd..9353bc292 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1115,4 +1115,86 @@ mod tests { assert!(!bot.client().skip_history_sync_enabled()); } + + // ── MessageContext Arc compatibility boundary ──────────────────────────── + // + // Lock down the contract introduced by switching `message` from Box to Arc. + // These tests guarantee: + // • `from_arc` doesn't deep-clone — `message_arc()` returns the same + // allocation (`Arc::ptr_eq`). + // • `message()` exposes the inner message read-only. + // • `into_box()` reclaims via `Arc::try_unwrap` when the Arc is unique + // (no clone), and falls back to deep-clone when shared. + async fn make_test_client() -> Arc { + let backend = create_test_sqlite_backend().await; + let bot = Bot::builder() + .with_backend(backend) + .with_transport_factory(TokioWebSocketTransportFactory::new()) + .with_http_client(MockHttpClient) + .with_runtime(TokioRuntime) + .build() + .await + .expect("Failed to build bot"); + bot.client() + } + + fn sample_message(text: &str) -> wa::Message { + wa::Message { + conversation: Some(text.to_string()), + ..Default::default() + } + } + + #[tokio::test] + async fn from_arc_preserves_allocation() { + let client = make_test_client().await; + let original = Arc::new(sample_message("ping")); + let original_ptr = Arc::as_ptr(&original); + + let ctx = MessageContext::from_arc(original, &MessageInfo::default(), client); + + assert_eq!(ctx.message().conversation.as_deref(), Some("ping")); + let exposed = ctx.message_arc(); + // Same allocation: the Arc round-trips without cloning the inner message. + assert!(std::ptr::eq(Arc::as_ptr(&exposed), original_ptr)); + } + + #[tokio::test] + async fn into_box_reclaims_when_unique() { + // When the inner Arc has refcount 1, `Arc::try_unwrap` succeeds and + // moves the message out without cloning. The Arc's allocation is then + // freed — observable via a `Weak` sentinel that becomes orphan. + // (Box::new still allocates fresh heap for the destination, so + // boxed.as_ref() addresses do not match the original Arc payload — + // hence we use Weak::strong_count, not pointer identity.) + let client = make_test_client().await; + let arc = Arc::new(sample_message("solo")); + let weak = Arc::downgrade(&arc); + let ctx = MessageContext::from_arc(arc, &MessageInfo::default(), client); + assert_eq!(weak.strong_count(), 1); + + let boxed = ctx.into_box(); + assert_eq!(boxed.conversation.as_deref(), Some("solo")); + assert_eq!(weak.strong_count(), 0); + assert!(weak.upgrade().is_none()); + } + + #[tokio::test] + async fn into_box_clones_when_shared() { + // With a second outstanding `Arc`, `try_unwrap` returns `Err` and + // `into_box` falls back to deep-cloning the inner message. The kept + // Arc survives with refcount 1 afterwards. + let client = make_test_client().await; + let arc = Arc::new(sample_message("shared")); + let kept = Arc::clone(&arc); + let ctx = MessageContext::from_arc(arc, &MessageInfo::default(), client); + assert_eq!(Arc::strong_count(&kept), 2); + + let boxed = ctx.into_box(); + assert_eq!(boxed.conversation.as_deref(), Some("shared")); + // ctx's internal Arc was dropped when into_box returned; only `kept` remains. + assert_eq!(Arc::strong_count(&kept), 1); + // Boxed message must be a distinct allocation from the surviving Arc. + assert!(!std::ptr::eq(boxed.as_ref() as *const _, Arc::as_ptr(&kept))); + } } From 8fb06bb8c7f9e0a2c6ad9e2e9b85f427e1ac2bb3 Mon Sep 17 00:00:00 2001 From: Salientekill Date: Sun, 3 May 2026 22:28:42 +0200 Subject: [PATCH 4/6] test(bot): path-level probe via CloneCounter + state tests renomeados MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit aponta corretamente que os testes anteriores (`into_box_reclaims_when_unique` / `_clones_when_shared`) só validavam o estado pós-into_box (Arc consumido / kept survives) — uma regressão hipotética que sempre clonasse passaria igual. Mudanças: - Renomeados os state-level tests pra refletir o que provam (`into_box_with_unique_arc_consumes_strong_ref` / `into_box_with_shared_arc_preserves_other_strong_refs`). - Comentários inline explicitam que são state-level e apontam pro path-level probe. - Novo módulo de teste algorítmico: - `CloneCounter`: payload com Drop side-effect via AtomicUsize que registra cada Clone::clone. - `try_unwrap_or_clone()`: réplica genérica do pattern de `into_box` (a fn de produção é monomorfizada em wa::Message). - `into_box_skips_clone_when_unique`: refcount==1 → counter == 0. - `into_box_invokes_clone_exactly_once_when_shared`: refcount>1 → counter == 1. Cobertura ironclad da fn de produção exigiria torná-la genérica; documentado inline como out-of-scope. Os tests algorítmicos garantem que o pattern descrito no PR se comporta como anunciado. --- src/bot.rs | 104 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 14 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index 9353bc292..e2c1c6dc0 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1160,13 +1160,12 @@ mod tests { } #[tokio::test] - async fn into_box_reclaims_when_unique() { - // When the inner Arc has refcount 1, `Arc::try_unwrap` succeeds and - // moves the message out without cloning. The Arc's allocation is then - // freed — observable via a `Weak` sentinel that becomes orphan. - // (Box::new still allocates fresh heap for the destination, so - // boxed.as_ref() addresses do not match the original Arc payload — - // hence we use Weak::strong_count, not pointer identity.) + async fn into_box_with_unique_arc_consumes_strong_ref() { + // State-level test: confirms that when the inner Arc has refcount 1, + // `into_box` consumes it (Weak sentinel becomes orphan). This alone + // does NOT prove `try_unwrap` was taken — a hypothetical impl that + // always clones and drops the original would pass the same way. + // Path-level coverage lives in `into_box_skips_clone_when_unique`. let client = make_test_client().await; let arc = Arc::new(sample_message("solo")); let weak = Arc::downgrade(&arc); @@ -1180,10 +1179,11 @@ mod tests { } #[tokio::test] - async fn into_box_clones_when_shared() { - // With a second outstanding `Arc`, `try_unwrap` returns `Err` and - // `into_box` falls back to deep-cloning the inner message. The kept - // Arc survives with refcount 1 afterwards. + async fn into_box_with_shared_arc_preserves_other_strong_refs() { + // State-level: with a second outstanding `Arc`, into_box drops only + // its own strong ref; `kept` survives at refcount 1 and the boxed + // message lives in a distinct allocation. Path-level (clone happens + // exactly once) covered in `into_box_skips_clone_when_unique`. let client = make_test_client().await; let arc = Arc::new(sample_message("shared")); let kept = Arc::clone(&arc); @@ -1192,9 +1192,85 @@ mod tests { let boxed = ctx.into_box(); assert_eq!(boxed.conversation.as_deref(), Some("shared")); - // ctx's internal Arc was dropped when into_box returned; only `kept` remains. assert_eq!(Arc::strong_count(&kept), 1); - // Boxed message must be a distinct allocation from the surviving Arc. - assert!(!std::ptr::eq(boxed.as_ref() as *const _, Arc::as_ptr(&kept))); + assert!(!std::ptr::eq( + boxed.as_ref() as *const _, + Arc::as_ptr(&kept) + )); + } + + // ── Path-level probe for the `into_box` algorithm ──────────────────────── + // + // The state-level tests above can't distinguish `try_unwrap → Ok` from a + // hypothetical "always clone, then drop the Arc" impl. This block probes + // the algorithm directly: a payload type that increments a shared counter + // every time `Clone::clone` runs. We replicate the exact `match + // Arc::try_unwrap(...) { Ok => Box::new, Err => clone }` pattern from + // `into_box` and assert the counter only changes on the shared path. + // + // A regression that swaps `into_box` for `Box::new((*self.message).clone())` + // would be caught by these tests if the algorithm were exercised; for + // ironclad coverage you'd need to make `into_box` itself generic over the + // payload type, which is out of scope here. Until then this guarantees + // the pattern this PR is shipping behaves as documented. + + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct CloneCounter { + count: Arc, + } + + impl Clone for CloneCounter { + fn clone(&self) -> Self { + self.count.fetch_add(1, Ordering::SeqCst); + Self { + count: Arc::clone(&self.count), + } + } + } + + fn try_unwrap_or_clone(arc: Arc) -> Box { + // Mirror of `MessageContext::into_box`. Kept here as a free function + // so it can be exercised against any T — the production version is + // monomorphized over `wa::Message` and can't be probed directly. + match Arc::try_unwrap(arc) { + Ok(v) => Box::new(v), + Err(arc) => Box::new((*arc).clone()), + } + } + + #[test] + fn into_box_skips_clone_when_unique() { + let counter = Arc::new(AtomicUsize::new(0)); + let payload = CloneCounter { + count: Arc::clone(&counter), + }; + let arc = Arc::new(payload); + + let _boxed = try_unwrap_or_clone(arc); + + assert_eq!( + counter.load(Ordering::SeqCst), + 0, + "try_unwrap path must not invoke Clone" + ); + } + + #[test] + fn into_box_invokes_clone_exactly_once_when_shared() { + let counter = Arc::new(AtomicUsize::new(0)); + let payload = CloneCounter { + count: Arc::clone(&counter), + }; + let arc = Arc::new(payload); + let _kept = Arc::clone(&arc); + + let _boxed = try_unwrap_or_clone(arc); + + assert_eq!( + counter.load(Ordering::SeqCst), + 1, + "shared-arc fallback must invoke Clone exactly once" + ); } } From 8c119eda2770dab1b5b10177015a742aa470f18c Mon Sep 17 00:00:00 2001 From: Salientekill Date: Sun, 3 May 2026 22:39:21 +0200 Subject: [PATCH 5/6] refactor(bot): single arc_into_box helper used by prod + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Endereça duas observações do CodeRabbit no PR #613: 1. (Major nitpick) Eliminar duplicação prod/test do algoritmo. Antes existia tanto MessageContext::into_box quanto try_unwrap_or_clone (em tests) com a mesma match Arc::try_unwrap. Helper privado genérico arc_into_box(arc) -> Box agora é a única fonte da verdade — into_box delega, e os path-level tests chamam o mesmo helper. Regressão na produção sai imediatamente nos tests. 2. (Minor) Stale ref no comentário do state-test 'shared'. Apontava pra into_box_skips_clone_when_unique; corrigido pra into_box_invokes_clone_exactly_once_when_shared (que é onde a garantia 'clone exactly once' é asserida). --- src/bot.rs | 46 +++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index e2c1c6dc0..7cd738712 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -30,6 +30,17 @@ pub enum BotBuilderError { Other(#[from] anyhow::Error), } +/// Single source of truth for the `Arc::try_unwrap` → `Box` conversion. +/// Used by [`MessageContext::into_box`] and exercised directly in tests so +/// production and test code share one implementation — no chance for a +/// regression in the production path to slip past tests that mirror it. +fn arc_into_box(arc: Arc) -> Box { + match Arc::try_unwrap(arc) { + Ok(value) => Box::new(value), + Err(arc) => Box::new((*arc).clone()), + } +} + /// `message` lives behind `Arc` so cloning the context (e.g., re-spawning into /// a fire-and-forget task) only bumps a refcount — deep-cloning per spawn was /// costly on hot paths (emoji-challenge, sticker triggers) where the message @@ -84,10 +95,7 @@ impl MessageContext { /// ownership. Tries to reclaim the inner `Message` when the `Arc` is /// uniquely held; falls back to deep-cloning otherwise. pub fn into_box(self) -> Box { - match Arc::try_unwrap(self.message) { - Ok(msg) => Box::new(msg), - Err(arc) => Box::new((*arc).clone()), - } + arc_into_box(self.message) } pub fn from_event(event: &Event, client: Arc) -> Option { @@ -1183,7 +1191,7 @@ mod tests { // State-level: with a second outstanding `Arc`, into_box drops only // its own strong ref; `kept` survives at refcount 1 and the boxed // message lives in a distinct allocation. Path-level (clone happens - // exactly once) covered in `into_box_skips_clone_when_unique`. + // exactly once) covered in `into_box_invokes_clone_exactly_once_when_shared`. let client = make_test_client().await; let arc = Arc::new(sample_message("shared")); let kept = Arc::clone(&arc); @@ -1203,16 +1211,10 @@ mod tests { // // The state-level tests above can't distinguish `try_unwrap → Ok` from a // hypothetical "always clone, then drop the Arc" impl. This block probes - // the algorithm directly: a payload type that increments a shared counter - // every time `Clone::clone` runs. We replicate the exact `match - // Arc::try_unwrap(...) { Ok => Box::new, Err => clone }` pattern from - // `into_box` and assert the counter only changes on the shared path. - // - // A regression that swaps `into_box` for `Box::new((*self.message).clone())` - // would be caught by these tests if the algorithm were exercised; for - // ironclad coverage you'd need to make `into_box` itself generic over the - // payload type, which is out of scope here. Until then this guarantees - // the pattern this PR is shipping behaves as documented. + // the algorithm directly: a payload type that bumps a shared counter on + // every `Clone::clone`, exercised against the SAME helper the production + // path uses. `MessageContext::into_box` delegates to `arc_into_box`, so a + // regression in the production path is forced to surface here. use std::sync::atomic::{AtomicUsize, Ordering}; @@ -1229,16 +1231,6 @@ mod tests { } } - fn try_unwrap_or_clone(arc: Arc) -> Box { - // Mirror of `MessageContext::into_box`. Kept here as a free function - // so it can be exercised against any T — the production version is - // monomorphized over `wa::Message` and can't be probed directly. - match Arc::try_unwrap(arc) { - Ok(v) => Box::new(v), - Err(arc) => Box::new((*arc).clone()), - } - } - #[test] fn into_box_skips_clone_when_unique() { let counter = Arc::new(AtomicUsize::new(0)); @@ -1247,7 +1239,7 @@ mod tests { }; let arc = Arc::new(payload); - let _boxed = try_unwrap_or_clone(arc); + let _boxed = arc_into_box(arc); assert_eq!( counter.load(Ordering::SeqCst), @@ -1265,7 +1257,7 @@ mod tests { let arc = Arc::new(payload); let _kept = Arc::clone(&arc); - let _boxed = try_unwrap_or_clone(arc); + let _boxed = arc_into_box(arc); assert_eq!( counter.load(Ordering::SeqCst), From 5826fe626c105036ada8a87a0f43182ada3d5d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= Date: Tue, 5 May 2026 09:54:57 -0300 Subject: [PATCH 6/6] perf(events): share wa::Message via Arc end-to-end Move `Event::Message` from `Box` to `Arc` and route `MessageContext::from_event` through `Arc::clone`, eliminating the deep-clone that previously happened on every dispatched message. Also trims the `MessageContext` API back to what's actually used in-tree: - field stays `pub` (no accessor methods); `Arc::clone(&ctx.message)` is the natural way to share, matching serenity / matrix-sdk - drops `into_box` and `arc_into_box` helper (no callers) - single test verifying `from_arc` doesn't deep-clone (Arc::as_ptr equality) Updated dispatch sites: `src/message.rs`, `src/pdo.rs` now `Arc::new` instead of `Box::new`. `Event::as_message` returns `&Arc` so callers that need the Arc can clone cheaply; auto-deref keeps existing `msg.field` access compiling. --- src/bot.rs | 191 +++---------------------------------- src/message.rs | 2 +- src/pdo.rs | 2 +- wacore/src/types/events.rs | 4 +- 4 files changed, 18 insertions(+), 181 deletions(-) diff --git a/src/bot.rs b/src/bot.rs index 7cd738712..c6094e002 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -30,43 +30,21 @@ pub enum BotBuilderError { Other(#[from] anyhow::Error), } -/// Single source of truth for the `Arc::try_unwrap` → `Box` conversion. -/// Used by [`MessageContext::into_box`] and exercised directly in tests so -/// production and test code share one implementation — no chance for a -/// regression in the production path to slip past tests that mirror it. -fn arc_into_box(arc: Arc) -> Box { - match Arc::try_unwrap(arc) { - Ok(value) => Box::new(value), - Err(arc) => Box::new((*arc).clone()), - } -} - -/// `message` lives behind `Arc` so cloning the context (e.g., re-spawning into -/// a fire-and-forget task) only bumps a refcount — deep-cloning per spawn was -/// costly on hot paths (emoji-challenge, sticker triggers) where the message -/// carries media buffers. -/// -/// The field is private to keep the storage representation an implementation -/// detail. Use [`Self::message`] for read-only access (95% of call sites), -/// [`Self::message_arc`] when you need to share ownership (e.g., re-spawn), -/// or [`Self::into_box`] for legacy code that requires `Box`. +/// `message` is `Arc` so cloning the context across spawned tasks only bumps a +/// refcount, matching the pattern used by serenity's `Context` and matrix-sdk's +/// `Room`/`Client`. #[derive(Clone)] pub struct MessageContext { - message: Arc, + pub message: Arc, pub info: MessageInfo, pub client: Arc, } impl MessageContext { pub fn from_parts(message: &wa::Message, info: &MessageInfo, client: Arc) -> Self { - Self { - message: Arc::new(message.clone()), - info: info.clone(), - client, - } + Self::from_arc(Arc::new(message.clone()), info, client) } - /// Zero-clone alternative when the caller already owns an `Arc`. pub fn from_arc(message: Arc, info: &MessageInfo, client: Arc) -> Self { Self { message, @@ -75,32 +53,9 @@ impl MessageContext { } } - /// Read-only access to the underlying message. Replaces direct - /// `ctx.message` field access from the previous `Box`-based API. - #[inline] - pub fn message(&self) -> &wa::Message { - &self.message - } - - /// Cheap clone of the inner `Arc` for sharing ownership across tasks - /// without deep-copying `wa::Message` (typical for [`Self::from_arc`] - /// re-spawns). Calling `Arc::clone` directly via `&ctx.message` is no - /// longer available since the field is private. - #[inline] - pub fn message_arc(&self) -> Arc { - Arc::clone(&self.message) - } - - /// Compatibility helper for legacy code that needs `Box` - /// ownership. Tries to reclaim the inner `Message` when the `Arc` is - /// uniquely held; falls back to deep-cloning otherwise. - pub fn into_box(self) -> Box { - arc_into_box(self.message) - } - pub fn from_event(event: &Event, client: Arc) -> Option { let (msg, info) = event.as_message()?; - Some(Self::from_parts(msg, info, client)) + Some(Self::from_arc(Arc::clone(msg), info, client)) } pub async fn send_message( @@ -1124,16 +1079,8 @@ mod tests { assert!(!bot.client().skip_history_sync_enabled()); } - // ── MessageContext Arc compatibility boundary ──────────────────────────── - // - // Lock down the contract introduced by switching `message` from Box to Arc. - // These tests guarantee: - // • `from_arc` doesn't deep-clone — `message_arc()` returns the same - // allocation (`Arc::ptr_eq`). - // • `message()` exposes the inner message read-only. - // • `into_box()` reclaims via `Arc::try_unwrap` when the Arc is unique - // (no clone), and falls back to deep-clone when shared. - async fn make_test_client() -> Arc { + #[tokio::test] + async fn from_arc_does_not_deep_clone() { let backend = create_test_sqlite_backend().await; let bot = Bot::builder() .with_backend(backend) @@ -1143,126 +1090,16 @@ mod tests { .build() .await .expect("Failed to build bot"); - bot.client() - } - fn sample_message(text: &str) -> wa::Message { - wa::Message { - conversation: Some(text.to_string()), + let original = Arc::new(wa::Message { + conversation: Some("ping".to_string()), ..Default::default() - } - } - - #[tokio::test] - async fn from_arc_preserves_allocation() { - let client = make_test_client().await; - let original = Arc::new(sample_message("ping")); + }); let original_ptr = Arc::as_ptr(&original); - let ctx = MessageContext::from_arc(original, &MessageInfo::default(), client); - - assert_eq!(ctx.message().conversation.as_deref(), Some("ping")); - let exposed = ctx.message_arc(); - // Same allocation: the Arc round-trips without cloning the inner message. - assert!(std::ptr::eq(Arc::as_ptr(&exposed), original_ptr)); - } - - #[tokio::test] - async fn into_box_with_unique_arc_consumes_strong_ref() { - // State-level test: confirms that when the inner Arc has refcount 1, - // `into_box` consumes it (Weak sentinel becomes orphan). This alone - // does NOT prove `try_unwrap` was taken — a hypothetical impl that - // always clones and drops the original would pass the same way. - // Path-level coverage lives in `into_box_skips_clone_when_unique`. - let client = make_test_client().await; - let arc = Arc::new(sample_message("solo")); - let weak = Arc::downgrade(&arc); - let ctx = MessageContext::from_arc(arc, &MessageInfo::default(), client); - assert_eq!(weak.strong_count(), 1); - - let boxed = ctx.into_box(); - assert_eq!(boxed.conversation.as_deref(), Some("solo")); - assert_eq!(weak.strong_count(), 0); - assert!(weak.upgrade().is_none()); - } - - #[tokio::test] - async fn into_box_with_shared_arc_preserves_other_strong_refs() { - // State-level: with a second outstanding `Arc`, into_box drops only - // its own strong ref; `kept` survives at refcount 1 and the boxed - // message lives in a distinct allocation. Path-level (clone happens - // exactly once) covered in `into_box_invokes_clone_exactly_once_when_shared`. - let client = make_test_client().await; - let arc = Arc::new(sample_message("shared")); - let kept = Arc::clone(&arc); - let ctx = MessageContext::from_arc(arc, &MessageInfo::default(), client); - assert_eq!(Arc::strong_count(&kept), 2); - - let boxed = ctx.into_box(); - assert_eq!(boxed.conversation.as_deref(), Some("shared")); - assert_eq!(Arc::strong_count(&kept), 1); - assert!(!std::ptr::eq( - boxed.as_ref() as *const _, - Arc::as_ptr(&kept) - )); - } - - // ── Path-level probe for the `into_box` algorithm ──────────────────────── - // - // The state-level tests above can't distinguish `try_unwrap → Ok` from a - // hypothetical "always clone, then drop the Arc" impl. This block probes - // the algorithm directly: a payload type that bumps a shared counter on - // every `Clone::clone`, exercised against the SAME helper the production - // path uses. `MessageContext::into_box` delegates to `arc_into_box`, so a - // regression in the production path is forced to surface here. - - use std::sync::atomic::{AtomicUsize, Ordering}; - - struct CloneCounter { - count: Arc, - } - - impl Clone for CloneCounter { - fn clone(&self) -> Self { - self.count.fetch_add(1, Ordering::SeqCst); - Self { - count: Arc::clone(&self.count), - } - } - } + let ctx = + MessageContext::from_arc(Arc::clone(&original), &MessageInfo::default(), bot.client()); - #[test] - fn into_box_skips_clone_when_unique() { - let counter = Arc::new(AtomicUsize::new(0)); - let payload = CloneCounter { - count: Arc::clone(&counter), - }; - let arc = Arc::new(payload); - - let _boxed = arc_into_box(arc); - - assert_eq!( - counter.load(Ordering::SeqCst), - 0, - "try_unwrap path must not invoke Clone" - ); - } - - #[test] - fn into_box_invokes_clone_exactly_once_when_shared() { - let counter = Arc::new(AtomicUsize::new(0)); - let payload = CloneCounter { - count: Arc::clone(&counter), - }; - let arc = Arc::new(payload); - let _kept = Arc::clone(&arc); - - let _boxed = arc_into_box(arc); - - assert_eq!( - counter.load(Ordering::SeqCst), - 1, - "shared-arc fallback must invoke Clone exactly once" - ); + assert!(std::ptr::eq(Arc::as_ptr(&ctx.message), original_ptr)); } } diff --git a/src/message.rs b/src/message.rs index ebfc6eb4a..6949eb0e3 100644 --- a/src/message.rs +++ b/src/message.rs @@ -96,7 +96,7 @@ impl Client { self.core .event_bus - .dispatch(Event::Message(Box::new(msg), info)); + .dispatch(Event::Message(Arc::new(msg), info)); } /// Handles a newsletter plaintext message. diff --git a/src/pdo.rs b/src/pdo.rs index 03b011f12..a33728b71 100644 --- a/src/pdo.rs +++ b/src/pdo.rs @@ -370,7 +370,7 @@ impl Client { self.core .event_bus .dispatch(wacore::types::events::Event::Message( - Box::new(message), + Arc::new(message), message_info, )); } diff --git a/wacore/src/types/events.rs b/wacore/src/types/events.rs index 0213af0ab..14a6ea318 100755 --- a/wacore/src/types/events.rs +++ b/wacore/src/types/events.rs @@ -386,7 +386,7 @@ pub enum Event { QrScannedWithoutMultidevice(QrScannedWithoutMultidevice), ClientOutdated(ClientOutdated), - Message(Box, Arc), + Message(Arc, Arc), Receipt(Receipt), UndecryptableMessage(UndecryptableMessage), #[serde(skip)] @@ -466,7 +466,7 @@ pub struct MexNotification { } impl Event { - pub fn as_message(&self) -> Option<(&wa::Message, &MessageInfo)> { + pub fn as_message(&self) -> Option<(&Arc, &MessageInfo)> { if let Event::Message(msg, info) = self { Some((msg, &**info)) } else {