From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 346671FF0B3 for ; Wed, 09 Sep 2026 11:19:44 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C2E1F214FC; Wed, 09 Sep 2026 11:19:43 +0200 (CEST) From: Dominik Csapak To: yew-devel@lists.proxmox.com Subject: [PATCH yew-widget-toolkit] state: (tree)store: fix mutable borrow during listener notification Date: Wed, 9 Sep 2026 11:18:12 +0200 Message-ID: <20260909091935.3557894-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.495 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 7N2KWU6AG57FY3K5GKC6NIWO3FQZZNLL X-Message-ID-Hash: 7N2KWU6AG57FY3K5GKC6NIWO3FQZZNLL X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Yew framework devel list at Proxmox List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: In the (tree)store, when a write guard was dropped, the listener were notified while there was still a mutable borrow on it. This would lead to a panic when trying to access the tree or store inside the listener. To avoid that, use a similar pattern as we have e.g. for SharedState or Selection, and use ManuallyDrop. Then we can drop the borrow before notifying the listeners. Signed-off-by: Dominik Csapak --- inspired by the recent changes to the loader/asyncabortguard by thomas. src/state/store.rs | 49 +++++++++++++++++++++---- src/state/tree_store/keyed_slab_tree.rs | 7 ++-- src/state/tree_store/mod.rs | 43 ++++++++++++++++++++-- 3 files changed, 83 insertions(+), 16 deletions(-) diff --git a/src/state/store.rs b/src/state/store.rs index ec34f50..1afe0a4 100644 --- a/src/state/store.rs +++ b/src/state/store.rs @@ -1,4 +1,5 @@ use std::cell::{Ref, RefCell, RefMut}; +use std::mem::ManuallyDrop; use std::ops::Range; use std::ops::{Deref, DerefMut}; use std::rc::Rc; @@ -185,7 +186,7 @@ impl Store { /// being in place. pub fn try_write(&self) -> Result, Error> { Ok(StoreWriteGuard { - state: self.inner.try_borrow_mut()?, + state: ManuallyDrop::new(self.inner.try_borrow_mut()?), update: true, }) } @@ -277,7 +278,7 @@ impl Store { /// /// Notifies store listeners when dropped, except when using [StoreWriteGuard::skip_update] pub struct StoreWriteGuard<'a, T: 'static> { - state: RefMut<'a, StoreState>, + state: ManuallyDrop>>, update: bool, } @@ -306,9 +307,22 @@ impl DerefMut for StoreWriteGuard<'_, T> { impl Drop for StoreWriteGuard<'_, T> { fn drop(&mut self) { - if self.update { + let listeners = if self.update { self.version += 1; - self.state.notify_listeners(); + Some(self.state.clone_listeners()) + } else { + None + }; + + // SAFETY: Only called once (here), and the value is not used afterwards. + unsafe { + ManuallyDrop::drop(&mut self.state); + } + + if let Some(listeners) = listeners { + for (_key, listener) in listeners.iter() { + listener.emit(()); + } } } } @@ -495,10 +509,9 @@ impl StoreState { self.listeners.remove(key); } - pub(crate) fn notify_listeners(&self) { - for (_key, listener) in self.listeners.iter() { - listener.emit(()); - } + /// Snapshot of the listeners, so they can be notified after releasing the borrow. + pub(crate) fn clone_listeners(&self) -> Slab> { + self.listeners.clone() } fn set_sorter(&mut self, sorter: impl IntoSorterFn) { @@ -643,3 +656,23 @@ where Some((pos, node)) } } + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use super::*; + + #[test] + fn listener_can_read_store_during_notification() { + let store = Store::with_extract_key(ExtractKeyFn::new(|v: &u32| Key::from(*v as u64))); + let seen = Rc::new(Cell::new(0)); + let _observer = store.add_listener({ + let store = store.clone(); + let seen = Rc::clone(&seen); + move |()| seen.set(store.read().data().len()) + }); + store.set_data(vec![1, 2, 3]); + assert_eq!(seen.get(), 3); + } +} diff --git a/src/state/tree_store/keyed_slab_tree.rs b/src/state/tree_store/keyed_slab_tree.rs index 1ff0d84..11e93d8 100644 --- a/src/state/tree_store/keyed_slab_tree.rs +++ b/src/state/tree_store/keyed_slab_tree.rs @@ -279,10 +279,9 @@ impl KeyedSlabTree { self.listeners.remove(key); } - pub(crate) fn notify_listeners(&self) { - for (_key, listener) in self.listeners.iter() { - listener.emit(()); - } + /// Snapshot of the listeners, so they can be notified after releasing the borrow. + pub(crate) fn clone_listeners(&self) -> Slab> { + self.listeners.clone() } pub(crate) fn set_sorter(&mut self, sorter: impl IntoSorterFn) { diff --git a/src/state/tree_store/mod.rs b/src/state/tree_store/mod.rs index e45e1f9..5e84b8f 100644 --- a/src/state/tree_store/mod.rs +++ b/src/state/tree_store/mod.rs @@ -14,6 +14,7 @@ pub use keyed_slab_tree::{ mod slab_tree_serde; use std::cell::{Ref, RefCell, RefMut}; +use std::mem::ManuallyDrop; use std::ops::{Deref, DerefMut, Range}; use std::rc::Rc; @@ -143,7 +144,7 @@ impl TreeStore { /// /// Panics if the store is already locked. pub fn write(&self) -> TreeStoreWriteGuard<'_, T> { - let tree = self.inner.borrow_mut(); + let tree = ManuallyDrop::new(self.inner.borrow_mut()); TreeStoreWriteGuard { initial_version: tree.version(), @@ -316,7 +317,7 @@ impl Deref for TreeStoreReadGuard<'_, T> { /// A wrapper type for a mutably borrowed [TreeStore] pub struct TreeStoreWriteGuard<'a, T> { - tree: RefMut<'a, KeyedSlabTree>, + tree: ManuallyDrop>>, initial_version: usize, } @@ -336,8 +337,22 @@ impl DerefMut for TreeStoreWriteGuard<'_, T> { impl Drop for TreeStoreWriteGuard<'_, T> { fn drop(&mut self) { - if self.tree.version() != self.initial_version { - self.tree.notify_listeners(); + let changed = self.tree.version() != self.initial_version; + let listeners = if changed { + Some(self.tree.clone_listeners()) + } else { + None + }; + + // SAFETY: Only called once (here) and value is not used afterwards. + unsafe { + ManuallyDrop::drop(&mut self.tree); + } + + if let Some(listeners) = listeners { + for (_key, listener) in listeners.iter() { + listener.emit(()); + } } } } @@ -462,3 +477,23 @@ where Some((pos, node)) } } + +#[cfg(test)] +mod tests { + use std::cell::Cell; + + use super::*; + + #[test] + fn listener_can_read_store_during_notification() { + let store = TreeStore::with_extract_key(ExtractKeyFn::new(|v: &u32| Key::from(*v as u64))); + let seen = Rc::new(Cell::new(false)); + let _observer = store.add_listener({ + let store = store.clone(); + let seen = Rc::clone(&seen); + move |()| seen.set(store.read().root().is_some()) + }); + store.write().set_root(1); + assert!(seen.get()); + } +} -- 2.47.3