From 6a09b2f80bd90bc76bf3d8f94a3265906d381889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 22 Aug 2026 15:06:01 +0200 Subject: [PATCH] fix(ui-ios): pause mutations during touch scrolling --- benchmarks/ios-ui/issue_7763_touch_scroll.ts | 65 +++++++++++++ changelog.d/8609-ios-touch-scroll-mutation.md | 3 + crates/perry-ui-ios/src/app.rs | 6 ++ crates/perry-ui-ios/src/widgets/mod.rs | 1 + .../src/widgets/scroll_mutation_gate.rs | 69 ++++++++++++++ crates/perry-ui-ios/src/widgets/scrollview.rs | 91 +++++++++++++++---- .../tests/scroll_mutation_gate.rs | 4 + scripts/gc_runtime_root_holders.json | 2 +- 8 files changed, 221 insertions(+), 20 deletions(-) create mode 100644 benchmarks/ios-ui/issue_7763_touch_scroll.ts create mode 100644 changelog.d/8609-ios-touch-scroll-mutation.md create mode 100644 crates/perry-ui-ios/src/widgets/scroll_mutation_gate.rs create mode 100644 crates/perry-ui-ios/tests/scroll_mutation_gate.rs diff --git a/benchmarks/ios-ui/issue_7763_touch_scroll.ts b/benchmarks/ios-ui/issue_7763_touch_scroll.ts new file mode 100644 index 0000000000..44af4365cc --- /dev/null +++ b/benchmarks/ios-ui/issue_7763_touch_scroll.ts @@ -0,0 +1,65 @@ +// Reproduction for #7763: mutate a UIScrollView-hosted UIStackView while +// real touch-driven scrolling is active. Programmatic setContentOffset does +// not exercise the UIKit gesture/tracking path that triggers the crash. +// +// PERRY_NO_AUTO_OPTIMIZE=1 perry run ios \ +// benchmarks/ios-ui/issue_7763_touch_scroll.ts --device + +import { + App, + VStack, + Text, + ScrollView, + scrollviewSetChild, + widgetAddChild, + widgetClearChildren, + textSetString, + onFrame, +} from "perry/ui" + +const ROWS = 100 +const LIVE_LABELS = 50 +// Bound the structural churn so this isolates the UIKit crash instead of the +// separate retained-widget issue on branches that predate widget tombstones. +const CHURN_UNTIL_FRAME = 600 +const content = VStack(4, []) +const scroll = ScrollView() +scrollviewSetChild(scroll, content) + +const labels: unknown[] = [] +let frame = 0 + +function rebuild(): void { + widgetClearChildren(content) + labels.length = 0 + for (let i = 0; i < ROWS; i++) { + const row = Text(`row ${i} @ ${frame}`) + labels.push(row) + widgetAddChild(content, row) + } +} + +rebuild() + +function loop(): void { + frame++ + for (let i = 0; i < LIVE_LABELS; i++) { + textSetString(labels[i] as never, `row ${i} @ ${frame}`) + } + if (frame <= CHURN_UNTIL_FRAME && frame % 6 === 0) { + rebuild() + } + if (frame % 600 === 0) { + console.log(`issue-7763 frames: ${frame}`) + } + onFrame(loop) +} + +onFrame(loop) + +App({ + title: "Issue 7763 Touch Scroll", + width: 400, + height: 800, + body: VStack(0, [scroll]), +}) diff --git a/changelog.d/8609-ios-touch-scroll-mutation.md b/changelog.d/8609-ios-touch-scroll-mutation.md new file mode 100644 index 0000000000..933a886155 --- /dev/null +++ b/changelog.d/8609-ios-touch-scroll-mutation.md @@ -0,0 +1,3 @@ +### Fixed + +- iOS apps no longer abort inside UIKit layout when JavaScript mutates labels or arranged subviews while a `UIScrollView` is being dragged or decelerating. Perry now waits for touch scrolling and its final layout transaction to settle before resuming UI-producing runtime work. diff --git a/crates/perry-ui-ios/src/app.rs b/crates/perry-ui-ios/src/app.rs index 0d6aad9a02..44a086a0b5 100644 --- a/crates/perry-ui-ios/src/app.rs +++ b/crates/perry-ui-ios/src/app.rs @@ -637,6 +637,12 @@ define_class!( #[unsafe(method(pump:))] fn pump(&self, _sender: &AnyObject) { crate::catch_callback_panic("pump", std::panic::AssertUnwindSafe(|| { + // #7763: the default-mode timer resumes while UIScrollView is + // decelerating. Do not let JS-driven widget mutations overlap + // UIKit's touch-scroll layout transaction. + if crate::widgets::scrollview::defer_runtime_pump_for_touch_scroll() { + return; + } unsafe { js_callback_timer_tick(); js_interval_timer_tick(); diff --git a/crates/perry-ui-ios/src/widgets/mod.rs b/crates/perry-ui-ios/src/widgets/mod.rs index 719c72a645..a97782b322 100644 --- a/crates/perry-ui-ios/src/widgets/mod.rs +++ b/crates/perry-ui-ios/src/widgets/mod.rs @@ -22,6 +22,7 @@ pub mod progressview; pub mod qrcode; pub mod rich_text; pub mod rich_tooltip; +pub(crate) mod scroll_mutation_gate; pub mod scrollview; pub mod securefield; pub mod slider; diff --git a/crates/perry-ui-ios/src/widgets/scroll_mutation_gate.rs b/crates/perry-ui-ios/src/widgets/scroll_mutation_gate.rs new file mode 100644 index 0000000000..5906ee1001 --- /dev/null +++ b/crates/perry-ui-ios/src/widgets/scroll_mutation_gate.rs @@ -0,0 +1,69 @@ +use std::collections::HashSet; + +pub(crate) const POST_SCROLL_QUIET_PUMPS: u8 = 2; + +#[derive(Default)] +pub(crate) struct ScrollMutationGate { + active_scrolls: HashSet, + quiet_pumps_remaining: u8, +} + +impl ScrollMutationGate { + pub(crate) fn begin(&mut self, handle: i64) { + self.active_scrolls.insert(handle); + self.quiet_pumps_remaining = 0; + } + + pub(crate) fn end(&mut self, handle: i64) { + if self.active_scrolls.remove(&handle) && self.active_scrolls.is_empty() { + self.quiet_pumps_remaining = POST_SCROLL_QUIET_PUMPS; + } + } + + pub(crate) fn should_defer_pump(&mut self) -> bool { + if !self.active_scrolls.is_empty() { + return true; + } + if self.quiet_pumps_remaining > 0 { + self.quiet_pumps_remaining -= 1; + return true; + } + false + } +} + +#[cfg(test)] +mod tests { + use super::{ScrollMutationGate, POST_SCROLL_QUIET_PUMPS}; + + #[test] + fn touch_scroll_defers_until_after_quiet_pumps() { + let mut gate = ScrollMutationGate::default(); + assert!(!gate.should_defer_pump()); + + gate.begin(7); + assert!(gate.should_defer_pump()); + assert!(gate.should_defer_pump()); + + gate.end(7); + for _ in 0..POST_SCROLL_QUIET_PUMPS { + assert!(gate.should_defer_pump()); + } + assert!(!gate.should_defer_pump()); + } + + #[test] + fn overlapping_scrolls_keep_the_gate_closed() { + let mut gate = ScrollMutationGate::default(); + gate.begin(1); + gate.begin(2); + gate.end(1); + assert!(gate.should_defer_pump()); + + gate.end(2); + for _ in 0..POST_SCROLL_QUIET_PUMPS { + assert!(gate.should_defer_pump()); + } + assert!(!gate.should_defer_pump()); + } +} diff --git a/crates/perry-ui-ios/src/widgets/scrollview.rs b/crates/perry-ui-ios/src/widgets/scrollview.rs index 93cb98bc82..c02f9a11a4 100644 --- a/crates/perry-ui-ios/src/widgets/scrollview.rs +++ b/crates/perry-ui-ios/src/widgets/scrollview.rs @@ -7,6 +7,8 @@ use objc2_ui_kit::{UIScrollView, UIView}; use std::cell::RefCell; use std::collections::HashMap; +use super::scroll_mutation_gate::ScrollMutationGate; + extern "C" { fn js_closure_call0(closure: *const u8) -> f64; fn js_nanbox_get_pointer(value: f64) -> i64; @@ -85,6 +87,7 @@ pub fn create() -> i64 { let view: Retained = Retained::cast_unchecked(scroll); let handle = super::register_widget(view); + install_scroll_delegate(handle); #[cfg(feature = "geisterhand")] { extern "C" { @@ -237,9 +240,15 @@ pub fn end_refreshing(scroll_handle: i64) { } // ============================================================================= -// Issue #553 — onScrollEnd hook (infinite-scroll callback) +// UIScrollView delegate state (touch-scroll safety + onScrollEnd callback) // ============================================================================= +// #7763: UIKit can abort from its UIStackView layout pass when Perry mutates an +// arranged subview while a containing UIScrollView is decelerating. NSTimer's +// default run-loop mode already pauses during direct touch tracking, but it +// resumes for deceleration. Keep the JS/UI pump paused through the complete +// touch interaction and two subsequent pump turns so UIKit can commit the +// final scroll/layout transaction before another widget mutation. struct ScrollEndState { closure: f64, threshold_px: f64, @@ -248,11 +257,31 @@ struct ScrollEndState { thread_local! { static SCROLL_END_STATES: RefCell> = RefCell::new(HashMap::new()); - static SCROLL_DELEGATE_TO_HANDLE: RefCell> = RefCell::new(HashMap::new()); + static SCROLL_MUTATION_GATE: RefCell = RefCell::new(ScrollMutationGate::default()); +} + +static SCROLL_DELEGATE_ASSOCIATION_KEY: u8 = 0; + +fn begin_touch_scroll(handle: i64) { + if handle != 0 { + SCROLL_MUTATION_GATE.with(|gate| gate.borrow_mut().begin(handle)); + } +} + +fn end_touch_scroll(handle: i64) { + if handle != 0 { + SCROLL_MUTATION_GATE.with(|gate| gate.borrow_mut().end(handle)); + } +} + +/// Returns true while a touch-driven UIScrollView interaction can still have +/// pending UIKit layout work. Called once at the start of each runtime pump. +pub(crate) fn defer_runtime_pump_for_touch_scroll() -> bool { + SCROLL_MUTATION_GATE.with(|gate| gate.borrow_mut().should_defer_pump()) } pub struct PerryScrollEndDelegateIvars { - key: std::cell::Cell, + handle: std::cell::Cell, } define_class!( @@ -262,12 +291,26 @@ define_class!( pub struct PerryScrollEndDelegate; impl PerryScrollEndDelegate { + #[unsafe(method(scrollViewWillBeginDragging:))] + fn scroll_view_will_begin_dragging(&self, _scroll: &AnyObject) { + begin_touch_scroll(self.ivars().handle.get()); + } + + #[unsafe(method(scrollViewDidEndDragging:willDecelerate:))] + fn scroll_view_did_end_dragging(&self, _scroll: &AnyObject, decelerate: bool) { + if !decelerate { + end_touch_scroll(self.ivars().handle.get()); + } + } + + #[unsafe(method(scrollViewDidEndDecelerating:))] + fn scroll_view_did_end_decelerating(&self, _scroll: &AnyObject) { + end_touch_scroll(self.ivars().handle.get()); + } + #[unsafe(method(scrollViewDidScroll:))] fn scroll_view_did_scroll(&self, scroll: &AnyObject) { - let key = self.ivars().key.get(); - let handle = SCROLL_DELEGATE_TO_HANDLE.with(|m| { - m.borrow().get(&key).copied().unwrap_or(0) - }); + let handle = self.ivars().handle.get(); if handle == 0 { return; } unsafe { let offset: CGPoint = msg_send![scroll, contentOffset]; @@ -310,16 +353,36 @@ define_class!( impl PerryScrollEndDelegate { fn new() -> Retained { let this = Self::alloc().set_ivars(PerryScrollEndDelegateIvars { - key: std::cell::Cell::new(0), + handle: std::cell::Cell::new(0), }); unsafe { msg_send![super(this), init] } } } -pub fn set_scroll_end_callback(scroll_handle: i64, callback: f64, threshold_px: f64) { +fn install_scroll_delegate(scroll_handle: i64) { let Some(scroll_view) = super::get_widget(scroll_handle) else { return; }; + unsafe { + let delegate = PerryScrollEndDelegate::new(); + delegate.ivars().handle.set(scroll_handle); + let _: () = msg_send![&*scroll_view, setDelegate: &*delegate]; + // UIScrollView.delegate is weak. Associate the delegate with its view + // so it lives exactly as long as the scroll view instead of leaking a + // delegate for every created ScrollView. + objc2::ffi::objc_setAssociatedObject( + Retained::as_ptr(&scroll_view) as *mut AnyObject, + &SCROLL_DELEGATE_ASSOCIATION_KEY as *const u8 as *const std::ffi::c_void, + Retained::as_ptr(&delegate) as *mut AnyObject, + objc2::ffi::OBJC_ASSOCIATION_RETAIN_NONATOMIC, + ); + } +} + +pub fn set_scroll_end_callback(scroll_handle: i64, callback: f64, threshold_px: f64) { + if super::get_widget(scroll_handle).is_none() { + return; + } SCROLL_END_STATES.with(|s| { s.borrow_mut().insert( scroll_handle, @@ -334,14 +397,4 @@ pub fn set_scroll_end_callback(scroll_handle: i64, callback: f64, threshold_px: }, ); }); - unsafe { - let delegate = PerryScrollEndDelegate::new(); - let key = Retained::as_ptr(&delegate) as usize; - delegate.ivars().key.set(key); - SCROLL_DELEGATE_TO_HANDLE.with(|m| { - m.borrow_mut().insert(key, scroll_handle); - }); - let _: () = msg_send![&*scroll_view, setDelegate: &*delegate]; - std::mem::forget(delegate); - } } diff --git a/crates/perry-ui-ios/tests/scroll_mutation_gate.rs b/crates/perry-ui-ios/tests/scroll_mutation_gate.rs new file mode 100644 index 0000000000..b7b04dc088 --- /dev/null +++ b/crates/perry-ui-ios/tests/scroll_mutation_gate.rs @@ -0,0 +1,4 @@ +// The library is intentionally iOS-only. Include this pure state machine +// directly so its behavior can still be exercised by host CI. +#[path = "../src/widgets/scroll_mutation_gate.rs"] +mod scroll_mutation_gate; diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index da4ad77a7a..8c3cf49419 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -1722,7 +1722,7 @@ }, { "file": "crates/perry-ui-ios/src/widgets/scrollview.rs", - "name": "SCROLL_DELEGATE_TO_HANDLE" + "name": "SCROLL_MUTATION_GATE" }, { "file": "crates/perry-ui-ios/src/widgets/scrollview.rs",