Skip to content

Commit 5f054ae

Browse files
committed
fix: clone terminal Node before invoking f in DomSlot traversal
1 parent c2baa85 commit 5f054ae

1 file changed

Lines changed: 21 additions & 13 deletions

File tree

packages/yew/src/dom_bundle/position.rs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -202,26 +202,34 @@ impl DynamicDomSlot {
202202
// TODO: there could be some data structure that performs better here. E.g. a balanced tree
203203
// with parent pointers come to mind, but they are a bit fiddly to implement in rust
204204
//
205-
// We traverse via raw pointers to avoid Rc refcount overhead (clone + drop) per hop.
205+
// We traverse via raw pointers to avoid Rc refcount overhead (clone + drop) per hop, then
206+
// clone the terminal next-sibling out of the chain before invoking `f`. Invoking `f` with
207+
// no borrow held and no reliance on chain structure keeps the traversal sound: `f` runs
208+
// arbitrary code (panic drop glue, `gloo::console::error`, tracing subscribers) that
209+
// could, in principle, reassign a link in the chain and drop the last strong reference
210+
// to the RefCell we would otherwise still borrow from.
206211
//
207-
// SAFETY: All RefCells in the chain are valid for the duration of this traversal:
212+
// SAFETY: All RefCells visited by the loop remain live while we dereference them:
208213
// - `self.target` (Rc) is alive because `self` is borrowed
209214
// - Each DomSlot::Chained(DynamicDomSlot { target }) in the chain holds a strong Rc to the
210215
// next RefCell, so all links are transitively kept alive
211-
// - Yew is single-threaded and this function does not yield, so no mutable borrow (e.g.
212-
// from reassign()) can occur on any RefCell in the chain during traversal
216+
// - Yew is single-threaded and the loop body does not run user code, so no mutable borrow
217+
// (e.g. from reassign()) can occur on any RefCell in the chain during traversal
213218
// - Each RefCell::borrow() is dropped before advancing to the next hop
214-
let mut ptr: *const RefCell<DomSlot> = Rc::as_ptr(&self.target);
215-
loop {
216-
let cell = unsafe { &*ptr };
217-
let slot_ref = cell.borrow();
218-
match &slot_ref.variant {
219-
DomSlotVariant::Node(ref n) => break f(n.as_ref()),
220-
DomSlotVariant::Chained(ref chain) => {
221-
ptr = Rc::as_ptr(&chain.target);
219+
let node: Option<Node> = {
220+
let mut ptr: *const RefCell<DomSlot> = Rc::as_ptr(&self.target);
221+
loop {
222+
let cell = unsafe { &*ptr };
223+
let slot_ref = cell.borrow();
224+
match &slot_ref.variant {
225+
DomSlotVariant::Node(ref n) => break n.clone(),
226+
DomSlotVariant::Chained(ref chain) => {
227+
ptr = Rc::as_ptr(&chain.target);
228+
}
222229
}
223230
}
224-
}
231+
};
232+
f(node.as_ref())
225233
}
226234
}
227235

0 commit comments

Comments
 (0)