Skip to content

Commit ee9cd41

Browse files
committed
Fix stale state in callbacks when multiple events fire rapidly
UseReducerHandle previously cached a single Rc<T> snapshot at render time. When dispatch updated the shared RefCell, callback closures still held handles pointing to the old snapshot, causing stale reads. Replace the snapshot with a deref_history approach: Deref now clones the latest Rc<T> from the shared RefCell and accumulates it in a Vec so that returned references remain valid for the handle's lifetime. The history is deduplicated by Rc::ptr_eq to avoid growth from repeated reads and is reset on each re-render when a new handle is created. This avoids the unsound pattern from PR #3963, where the Ref guard from try_borrow() was dropped while a pointer derived from it was returned. That caused use-after-free when dispatch replaced the Rc (refcount 1) and the allocator reused the freed memory. A dedicated test exercises this exact sequence and confirms it no longer corrupts memory. Fixes #3796 Supersedes #3963
1 parent 0ec71a3 commit ee9cd41

File tree

3 files changed

+291
-9
lines changed

3 files changed

+291
-9
lines changed

packages/yew/src/functional/hooks/use_reducer.rs

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ pub struct UseReducerHandle<T>
3535
where
3636
T: Reducible,
3737
{
38-
value: Rc<T>,
38+
/// Shared source of truth, updated synchronously by dispatch.
39+
current_state: Rc<RefCell<Rc<T>>>,
40+
/// Accumulates `Rc<T>` clones returned by [`Deref::deref`] so that references
41+
/// remain valid for the lifetime of this handle. Reset on each re-render when
42+
/// a new handle is created.
43+
deref_history: RefCell<Vec<Rc<T>>>,
3944
dispatch: DispatchFn<T>,
4045
}
4146

@@ -63,7 +68,34 @@ where
6368
type Target = T;
6469

6570
fn deref(&self) -> &Self::Target {
66-
&self.value
71+
let rc = match self.current_state.try_borrow() {
72+
Ok(shared) => Rc::clone(&*shared),
73+
Err(_) => {
74+
// RefCell is mutably borrowed (during dispatch). Use the last
75+
// value we successfully read.
76+
let history = self.deref_history.borrow();
77+
Rc::clone(history.last().expect("deref_history is never empty"))
78+
}
79+
};
80+
81+
let ptr: *const T = Rc::as_ptr(&rc);
82+
83+
// Only store a new entry when the Rc allocation differs from the most
84+
// recent one, avoiding unbounded growth from repeated reads of the same
85+
// state.
86+
{
87+
let mut history = self.deref_history.borrow_mut();
88+
if !Rc::ptr_eq(history.last().expect("deref_history is never empty"), &rc) {
89+
history.push(rc);
90+
}
91+
}
92+
93+
// SAFETY: `ptr` points into the heap allocation of an `Rc<T>`. That Rc
94+
// is kept alive in `self.deref_history` (either the entry we just pushed,
95+
// or a previous entry with the same allocation). `deref_history` lives as
96+
// long as `self`, and `Rc` guarantees its heap allocation stays live while
97+
// any clone exists. Therefore `ptr` is valid for the lifetime of `&self`.
98+
unsafe { &*ptr }
6799
}
68100
}
69101

@@ -72,8 +104,17 @@ where
72104
T: Reducible,
73105
{
74106
fn clone(&self) -> Self {
107+
// Take a fresh snapshot so the clone's deref_history is never empty.
108+
let snapshot = match self.current_state.try_borrow() {
109+
Ok(shared) => Rc::clone(&*shared),
110+
Err(_) => {
111+
let history = self.deref_history.borrow();
112+
Rc::clone(history.last().expect("deref_history is never empty"))
113+
}
114+
};
75115
Self {
76-
value: Rc::clone(&self.value),
116+
current_state: Rc::clone(&self.current_state),
117+
deref_history: RefCell::new(vec![snapshot]),
77118
dispatch: Rc::clone(&self.dispatch),
78119
}
79120
}
@@ -84,8 +125,14 @@ where
84125
T: Reducible + fmt::Debug,
85126
{
86127
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
128+
let value = if let Ok(rc_ref) = self.current_state.try_borrow() {
129+
format!("{:?}", *rc_ref)
130+
} else {
131+
let history = self.deref_history.borrow();
132+
format!("{:?}", **history.last().expect("deref_history is never empty"))
133+
};
87134
f.debug_struct("UseReducerHandle")
88-
.field("value", &format!("{:?}", self.value))
135+
.field("value", &value)
89136
.finish()
90137
}
91138
}
@@ -95,7 +142,7 @@ where
95142
T: Reducible + PartialEq,
96143
{
97144
fn eq(&self, rhs: &Self) -> bool {
98-
self.value == rhs.value
145+
**self == **rhs
99146
}
100147
}
101148

@@ -239,10 +286,15 @@ where
239286
}
240287
});
241288

242-
let value = state.current_state.borrow().clone();
289+
let current_state = state.current_state.clone();
290+
let snapshot = state.current_state.borrow().clone();
243291
let dispatch = state.dispatch.clone();
244292

245-
UseReducerHandle { value, dispatch }
293+
UseReducerHandle {
294+
current_state,
295+
deref_history: RefCell::new(vec![snapshot]),
296+
dispatch,
297+
}
246298
}
247299
}
248300

packages/yew/src/functional/hooks/use_state.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ pub struct UseStateHandle<T> {
109109
impl<T: fmt::Debug> fmt::Debug for UseStateHandle<T> {
110110
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
111111
f.debug_struct("UseStateHandle")
112-
.field("value", &format!("{:?}", self.inner.value))
112+
.field("value", &format!("{:?}", **self))
113113
.finish()
114114
}
115115
}
@@ -132,7 +132,7 @@ impl<T> Deref for UseStateHandle<T> {
132132
type Target = T;
133133

134134
fn deref(&self) -> &Self::Target {
135-
&(*self.inner).value
135+
&self.inner.value
136136
}
137137
}
138138

packages/yew/tests/use_state.rs

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,233 @@ async fn use_state_eq_works() {
106106
assert_eq!(result.as_str(), "1");
107107
assert_eq!(RENDER_COUNT.load(Ordering::Relaxed), 2);
108108
}
109+
110+
/// Exercises the exact pattern that causes use-after-free in the original PR #3963
111+
/// fix, where `UseReducerHandle::deref()` drops the `Ref` guard but returns a
112+
/// pointer derived from it.
113+
///
114+
/// The dangerous sequence within a single callback:
115+
/// 1. `handle.set(v1)` — dispatch puts a *new* `Rc` (refcount=1) in the shared
116+
/// `RefCell`, replacing the one from render time.
117+
/// 2. `let r: &T = &*handle` — `deref()` borrows the RefCell, grabs a raw pointer
118+
/// into the Rc (refcount still 1), and **drops the `Ref` guard**.
119+
/// 3. `handle.set(v2)` — dispatch replaces that Rc. Because its refcount was 1,
120+
/// it is freed. `r` is now dangling.
121+
/// 4. Allocate objects of similar size to encourage the allocator to reuse the
122+
/// freed memory, overwriting the old `T`.
123+
/// 5. Read through `r` — **use-after-free**.
124+
///
125+
/// With the `deref_history` fix, step 2 clones the Rc into a `Vec` kept alive by
126+
/// the handle, bumping the refcount to 2. Step 3 only drops it to 1, so the
127+
/// allocation survives and `r` remains valid.
128+
#[wasm_bindgen_test]
129+
async fn deref_remains_valid_across_multiple_dispatches_in_callback() {
130+
use std::cell::RefCell;
131+
132+
use gloo::utils::document;
133+
use wasm_bindgen::JsCast;
134+
use web_sys::HtmlElement;
135+
136+
thread_local! {
137+
static DEREF_RESULT: RefCell<Option<String>> = const { RefCell::new(None) };
138+
}
139+
140+
#[component(UBTestComponent)]
141+
fn ub_test_comp() -> Html {
142+
let state = use_state(|| "initial".to_string());
143+
144+
let trigger = {
145+
let state = state.clone();
146+
Callback::from(move |_| {
147+
// Step 1: dispatch. The RefCell now contains a *new* Rc whose only
148+
// owner is the RefCell itself (refcount = 1).
149+
state.set("first_dispatch".to_string());
150+
151+
// Step 2: deref. In the original fix the Ref guard is dropped
152+
// immediately, leaving us with a bare pointer into the refcount-1
153+
// Rc. With deref_history, the Rc is cloned into the Vec so the
154+
// refcount is bumped to 2.
155+
let borrowed: &String = &*state;
156+
157+
// Step 3: dispatch again. The RefCell's old Rc is replaced.
158+
// Original fix: refcount was 1 → drops to 0 → freed → `borrowed`
159+
// dangles.
160+
// deref_history fix: refcount was 2 → drops to 1 (still in Vec)
161+
// → allocation survives → `borrowed` is valid.
162+
state.set("second_dispatch".to_string());
163+
164+
// Step 4: churn the allocator. Create and drop many heap objects
165+
// of ~32 bytes (the size of the freed Rc+UseStateReducer+String
166+
// struct on wasm32) to maximize the chance that the allocator
167+
// hands out the freed address to one of these, overwriting the
168+
// memory `borrowed` points into.
169+
for _ in 0..256 {
170+
// Each Box<[u8; 32]> is roughly the same size as the freed Rc
171+
// allocation containing UseStateReducer<String>.
172+
let overwrite = Box::new([0xFFu8; 32]);
173+
std::hint::black_box(&*overwrite);
174+
drop(overwrite);
175+
}
176+
177+
// Also allocate Strings whose *buffers* might reuse the freed
178+
// String buffer from step 1.
179+
let _noise: Vec<String> = (0..64)
180+
.map(|i| format!("noise_{:032}", i))
181+
.collect();
182+
183+
// Step 5: read through the potentially-dangling reference.
184+
// With the original fix this is UB: the memory behind `borrowed`
185+
// may have been reused by the allocations above, so `.clone()`
186+
// could read a garbage ptr/len/cap triple and trap, or silently
187+
// return corrupted data.
188+
// With deref_history, this always reads "first_dispatch".
189+
let value = borrowed.clone();
190+
191+
DEREF_RESULT.with(|r| {
192+
*r.borrow_mut() = Some(value);
193+
});
194+
})
195+
};
196+
197+
html! {
198+
<div>
199+
<button id="ub-trigger" onclick={trigger}>{"Trigger"}</button>
200+
<div id="result">{(*state).clone()}</div>
201+
</div>
202+
}
203+
}
204+
205+
yew::Renderer::<UBTestComponent>::with_root(
206+
document().get_element_by_id("output").unwrap(),
207+
)
208+
.render();
209+
sleep(Duration::ZERO).await;
210+
211+
// Fire the callback
212+
document()
213+
.get_element_by_id("ub-trigger")
214+
.unwrap()
215+
.unchecked_into::<HtmlElement>()
216+
.click();
217+
218+
sleep(Duration::ZERO).await;
219+
220+
// The reference obtained between the two dispatches must still read the
221+
// value from the first dispatch, not garbage or "second_dispatch".
222+
let captured = DEREF_RESULT.with(|r| r.borrow().clone());
223+
assert_eq!(
224+
captured,
225+
Some("first_dispatch".to_string()),
226+
"deref() reference must remain valid across subsequent dispatches"
227+
);
228+
}
229+
230+
/// Regression test for issue #3796
231+
/// Tests that state handles always read the latest value even when accessed
232+
/// from callbacks before a rerender occurs.
233+
///
234+
/// The bug occurred when:
235+
/// 1. State A is updated via set()
236+
/// 2. State B is updated via set()
237+
/// 3. A callback reads both states before rerender
238+
/// 4. The callback would see stale value for B because the handle was caching a snapshot instead of
239+
/// reading from the shared RefCell
240+
#[wasm_bindgen_test]
241+
async fn use_state_handles_read_latest_value_issue_3796() {
242+
use std::cell::RefCell;
243+
244+
use gloo::utils::document;
245+
use wasm_bindgen::JsCast;
246+
use web_sys::HtmlElement;
247+
248+
// Shared storage for the values read by the submit handler
249+
thread_local! {
250+
static CAPTURED_VALUES: RefCell<Option<(String, String)>> = const { RefCell::new(None) };
251+
}
252+
253+
#[component(FormComponent)]
254+
fn form_comp() -> Html {
255+
let field_a = use_state(String::new);
256+
let field_b = use_state(String::new);
257+
258+
let update_a = {
259+
let field_a = field_a.clone();
260+
Callback::from(move |_| {
261+
field_a.set("value_a".to_string());
262+
})
263+
};
264+
265+
let update_b = {
266+
let field_b = field_b.clone();
267+
Callback::from(move |_| {
268+
field_b.set("value_b".to_string());
269+
})
270+
};
271+
272+
// This callback reads both states - the bug caused field_b to be stale
273+
let submit = {
274+
let field_a = field_a.clone();
275+
let field_b = field_b.clone();
276+
Callback::from(move |_| {
277+
let a = (*field_a).clone();
278+
let b = (*field_b).clone();
279+
CAPTURED_VALUES.with(|v| {
280+
*v.borrow_mut() = Some((a.clone(), b.clone()));
281+
});
282+
})
283+
};
284+
285+
html! {
286+
<div>
287+
<button id="update-a" onclick={update_a}>{"Update A"}</button>
288+
<button id="update-b" onclick={update_b}>{"Update B"}</button>
289+
<button id="submit" onclick={submit}>{"Submit"}</button>
290+
<div id="result">{format!("a={}, b={}", *field_a, *field_b)}</div>
291+
</div>
292+
}
293+
}
294+
295+
yew::Renderer::<FormComponent>::with_root(document().get_element_by_id("output").unwrap())
296+
.render();
297+
sleep(Duration::ZERO).await;
298+
299+
// Initial state
300+
let result = obtain_result();
301+
assert_eq!(result.as_str(), "a=, b=");
302+
303+
// Click update-a, then update-b, then submit WITHOUT waiting for rerender.
304+
// This simulates rapid user interaction (like the Firefox bug in issue #3796).
305+
document()
306+
.get_element_by_id("update-a")
307+
.unwrap()
308+
.unchecked_into::<HtmlElement>()
309+
.click();
310+
311+
document()
312+
.get_element_by_id("update-b")
313+
.unwrap()
314+
.unchecked_into::<HtmlElement>()
315+
.click();
316+
317+
document()
318+
.get_element_by_id("submit")
319+
.unwrap()
320+
.unchecked_into::<HtmlElement>()
321+
.click();
322+
323+
// Now wait for rerenders to complete
324+
sleep(Duration::ZERO).await;
325+
326+
// Check the values captured by the submit handler.
327+
// Before the fix, field_b would be empty because the callback captured a stale handle.
328+
let captured = CAPTURED_VALUES.with(|v| v.borrow().clone());
329+
assert_eq!(
330+
captured,
331+
Some(("value_a".to_string(), "value_b".to_string())),
332+
"Submit handler should see latest values for both fields"
333+
);
334+
335+
// Also verify the DOM shows correct values after rerender
336+
let result = obtain_result();
337+
assert_eq!(result.as_str(), "a=value_a, b=value_b");
338+
}

0 commit comments

Comments
 (0)