Skip to content

Commit 67f073a

Browse files
committed
fix(studio): survive render panics without freezing the preview
Investigation of an app freeze + OOM while playing dynamic-glass with the worker pool. Isolated soaks exonerate the native pipeline: 6 workers at 0.5/1.0 scale hold a flat RSS (124/368 MB over 360 heavy frames), and a 60s full-pipeline soak (moving 30fps playhead, simulated asset handler, canvas hit pass) stays bounded at 122-482 MB with 3 cache misses in 1200 served frames. The identified failure mode is a panic cascade: the global cosmic-text mutexes were locked with .unwrap(), so one Skia panic on any render thread (caught by the panic fences) poisoned FONT_SYSTEM/SWASH_CACHE and turned every subsequent text shape on every thread into a panic — preview permanently stuck, six workers re-rendering panicking frames in a tight loop. - cosmic: poison-tolerant font/swash locks + regression test that deliberately poisons the mutex and shapes through it - studio: FailLedger — a frame whose render panics gets MAX_RENDER_ATTEMPTS tries (workers and asset handler), then is left alone instead of looping - keep the ignored stress/soak diagnostics for future perf work
1 parent d7ca65f commit 67f073a

4 files changed

Lines changed: 255 additions & 8 deletions

File tree

crates/rustmotion-core/src/engine/text/cosmic.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ pub fn measure_text(text: &str, style: &TextStyle) -> TextMetrics {
9696
height: metrics_for(style).line_height,
9797
};
9898
}
99-
let mut fs = font_system().lock().unwrap();
99+
// Poison-tolerant: a panic on another render thread (e.g. a Skia panic
100+
// caught by a preview worker's panic fence) must not poison text shaping
101+
// for every subsequent frame — the FontSystem stays usable, each shape
102+
// call builds its own Buffer.
103+
let mut fs = font_system().lock().unwrap_or_else(|e| e.into_inner());
100104
let metrics = metrics_for(style);
101105
let mut buf = Buffer::new(&mut fs, metrics);
102106
buf.set_size(style.max_width, None);
@@ -128,15 +132,16 @@ pub fn paint_text(
128132
if text.is_empty() {
129133
return;
130134
}
131-
let mut fs = font_system().lock().unwrap();
135+
// Poison-tolerant for the same reason as in `measure_text`.
136+
let mut fs = font_system().lock().unwrap_or_else(|e| e.into_inner());
132137
let metrics = metrics_for(style);
133138
let mut buf = Buffer::new(&mut fs, metrics);
134139
buf.set_size(style.max_width, None);
135140
buf.set_wrap(if style.wrap { Wrap::Word } else { Wrap::None });
136141
buf.set_text(text, &attrs_for(style), Shaping::Advanced, None);
137142
buf.shape_until_scroll(&mut fs, false);
138143

139-
let mut sc = swash_cache().lock().unwrap();
144+
let mut sc = swash_cache().lock().unwrap_or_else(|e| e.into_inner());
140145
let ccolor = CColor::rgba(color.r(), color.g(), color.b(), color.a());
141146

142147
for run in buf.layout_runs() {
@@ -264,4 +269,20 @@ mod tests {
264269
let m = measure_text("the quick brown fox", &style);
265270
assert!(m.height < 16.0 * 1.2 * 2.0, "should be one line");
266271
}
272+
273+
/// A panic on another thread while it holds the font mutex (e.g. a Skia
274+
/// panic caught by a preview worker's panic fence) poisons the lock.
275+
/// Shaping must survive that — otherwise one panic turns every subsequent
276+
/// text render on every thread into a panic cascade.
277+
#[test]
278+
fn measure_survives_poisoned_font_mutex() {
279+
let _ = std::thread::spawn(|| {
280+
let _guard = font_system().lock().unwrap_or_else(|e| e.into_inner());
281+
panic!("deliberate poison");
282+
})
283+
.join();
284+
assert!(font_system().is_poisoned(), "setup must have poisoned");
285+
let m = measure_text("still shaping", &TextStyle::default());
286+
assert!(m.width > 0.0, "measure works despite the poisoned mutex");
287+
}
267288
}

crates/rustmotion-studio/src/editor/frames.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,55 @@ mod tests {
185185
assert_eq!(&jpeg[0..2], &[0xFF, 0xD8], "must be a JPEG");
186186
}
187187

188+
/// TEMP diagnostic (not part of CI): replicate the prefetch worker pool on
189+
/// the dynamic-glass example and print RSS growth. Run with
190+
/// `RM_STRESS_SCALE=0.5 cargo test -p rustmotion-studio --release stress_rss -- --ignored --nocapture`
191+
#[test]
192+
#[ignore]
193+
fn stress_rss_worker_pool() {
194+
use std::sync::Arc;
195+
let Ok(src) = std::fs::read_to_string("../../examples/dynamic-glass.json") else {
196+
eprintln!("skipped: examples/dynamic-glass.json not present");
197+
return;
198+
};
199+
let scenario =
200+
Arc::new(rustmotion::loader::load_scenario_from_source(None, Some(&src)).unwrap());
201+
let tasks = Arc::new(rustmotion::encode::build_frame_tasks(&scenario));
202+
let scale: f32 = std::env::var("RM_STRESS_SCALE")
203+
.ok()
204+
.and_then(|s| s.parse().ok())
205+
.unwrap_or(0.5);
206+
let rss_kb = || -> u64 {
207+
let out = std::process::Command::new("ps")
208+
.args(["-o", "rss=", "-p", &std::process::id().to_string()])
209+
.output()
210+
.unwrap();
211+
String::from_utf8_lossy(&out.stdout).trim().parse().unwrap()
212+
};
213+
println!("scale={scale} start rss={} MB", rss_kb() / 1024);
214+
let workers: Vec<_> = (0..6u32)
215+
.map(|w| {
216+
let s = scenario.clone();
217+
let t = tasks.clone();
218+
std::thread::spawn(move || {
219+
for _pass in 0..6 {
220+
for f in (130..190).filter(|f| f % 6 == w) {
221+
let _ = render_frame(&s, &t, f, scale);
222+
}
223+
}
224+
})
225+
})
226+
.collect();
227+
while workers.iter().any(|w| !w.is_finished()) {
228+
std::thread::sleep(std::time::Duration::from_secs(2));
229+
println!("rss={} MB", rss_kb() / 1024);
230+
}
231+
for w in workers {
232+
w.join().unwrap();
233+
}
234+
println!("end rss={} MB", rss_kb() / 1024);
235+
}
236+
188237
#[test]
189238
fn renders_frame_at_reduced_scale() {
190239
let scenario = rustmotion::loader::load_scenario_from_source(None, Some(SCENARIO)).unwrap();

crates/rustmotion-studio/src/editor/prefetch.rs

Lines changed: 165 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,39 @@ fn claims() -> &'static Mutex<ClaimSet> {
206206
SLOT.get_or_init(|| Mutex::new(ClaimSet::default()))
207207
}
208208

209+
// ── Failure ledger ───────────────────────────────────────────────────────────
210+
211+
/// Attempts a frame gets before workers and the handler stop re-rendering it.
212+
/// One retry absorbs transient panics; beyond that a panicking frame must not
213+
/// become an infinite render-panic loop that pegs the workers (the preview
214+
/// shows the neighboring frame instead).
215+
pub const MAX_RENDER_ATTEMPTS: u8 = 2;
216+
217+
/// Panicked render attempts per key. Keys carry the generation, so a model
218+
/// edit naturally retires old entries; the size cap is a runaway backstop.
219+
#[derive(Default)]
220+
pub struct FailLedger {
221+
map: HashMap<FrameKey, u8>,
222+
}
223+
224+
impl FailLedger {
225+
pub fn record_failure(&mut self, key: FrameKey) {
226+
if self.map.len() > 4096 {
227+
self.map.clear();
228+
}
229+
*self.map.entry(key).or_insert(0) += 1;
230+
}
231+
232+
pub fn exhausted(&self, key: &FrameKey) -> bool {
233+
self.map.get(key).is_some_and(|&n| n >= MAX_RENDER_ATTEMPTS)
234+
}
235+
}
236+
237+
pub(crate) fn fail_ledger() -> &'static Mutex<FailLedger> {
238+
static SLOT: OnceLock<Mutex<FailLedger>> = OnceLock::new();
239+
SLOT.get_or_init(|| Mutex::new(FailLedger::default()))
240+
}
241+
209242
/// Prefetch worker count for a machine with `cores` logical cores: leave two
210243
/// for the UI/webview and the asset handler, keep at least two workers so a
211244
/// slow frame never serializes the window, and cap at six (JPEG frames past
@@ -324,6 +357,15 @@ fn prefetch_loop() {
324357
{
325358
continue;
326359
}
360+
// A frame that keeps panicking gets MAX_RENDER_ATTEMPTS, then is
361+
// left alone — never an infinite render-panic loop.
362+
if fail_ledger()
363+
.lock()
364+
.unwrap_or_else(|e| e.into_inner())
365+
.exhausted(&key)
366+
{
367+
continue;
368+
}
327369
// Another worker already rendering this frame → take the next one.
328370
if !claims()
329371
.lock()
@@ -344,11 +386,15 @@ fn prefetch_loop() {
344386
let rendered = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
345387
render_frame(&scenario, &tasks, frame, scale_factor(scale))
346388
}));
347-
if let Ok(bytes) = rendered {
348-
frame_cache()
389+
match rendered {
390+
Ok(bytes) => frame_cache()
391+
.lock()
392+
.unwrap_or_else(|e| e.into_inner())
393+
.insert(key, bytes, gen_b, gen_a, target.current, scale),
394+
Err(_) => fail_ledger()
349395
.lock()
350396
.unwrap_or_else(|e| e.into_inner())
351-
.insert(key, bytes, gen_b, gen_a, target.current, scale);
397+
.record_failure(key),
352398
}
353399
}
354400
claims()
@@ -623,6 +669,122 @@ mod tests {
623669
assert!(claims.try_claim(other_scale), "other scale");
624670
}
625671

672+
/// TEMP diagnostic (not CI): full-pipeline soak — real worker pool, a
673+
/// simulated 30fps playhead publishing targets, a simulated asset handler
674+
/// serving frames, and the canvas hit-map pass. Prints RSS + serve stats.
675+
/// `cargo test -p rustmotion-studio --release soak_full -- --ignored --nocapture`
676+
#[test]
677+
#[ignore]
678+
fn soak_full_pipeline_rss() {
679+
use crate::editor::frames::frame_hits;
680+
use std::sync::atomic::{AtomicU32, Ordering};
681+
682+
let Ok(src) = std::fs::read_to_string("../../examples/dynamic-glass.json") else {
683+
eprintln!("skipped: examples/dynamic-glass.json not present");
684+
return;
685+
};
686+
let scenario =
687+
Arc::new(rustmotion::loader::load_scenario_from_source(None, Some(&src)).unwrap());
688+
let tasks = Arc::new(rustmotion::encode::build_frame_tasks(&scenario));
689+
let total = tasks.len() as u32;
690+
let rss_mb = || -> u64 {
691+
let out = std::process::Command::new("ps")
692+
.args(["-o", "rss=", "-p", &std::process::id().to_string()])
693+
.output()
694+
.unwrap();
695+
String::from_utf8_lossy(&out.stdout)
696+
.trim()
697+
.parse::<u64>()
698+
.unwrap()
699+
/ 1024
700+
};
701+
702+
ensure_prefetcher();
703+
static SERVED: AtomicU32 = AtomicU32::new(0);
704+
static MISSES: AtomicU32 = AtomicU32::new(0);
705+
706+
// Simulated playback + asset handler + canvas hit pass, 30fps for 60s.
707+
let s2 = scenario.clone();
708+
let t2 = tasks.clone();
709+
let sim = std::thread::spawn(move || {
710+
let mut current = 0u32;
711+
for _tick in 0..(30 * 60) {
712+
std::thread::sleep(Duration::from_millis(33));
713+
current = (current + 1) % total;
714+
*prefetch_slot().lock().unwrap_or_else(|e| e.into_inner()) = PrefetchTarget {
715+
current,
716+
playing: true,
717+
generation: 1,
718+
side: DiffSide::B,
719+
scenario: Some(s2.clone()),
720+
tasks: Some(t2.clone()),
721+
path: None,
722+
};
723+
let key = FrameKey {
724+
generation: 1,
725+
side: DiffSide::B,
726+
frame: current,
727+
scale_pct: preview_scale_pct(),
728+
};
729+
let hit = frame_cache()
730+
.lock()
731+
.unwrap_or_else(|e| e.into_inner())
732+
.contains(&key);
733+
if !hit {
734+
MISSES.fetch_add(1, Ordering::Relaxed);
735+
let bytes = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
736+
crate::editor::frames::render_frame(
737+
&s2,
738+
&t2,
739+
current,
740+
scale_factor(key.scale_pct),
741+
)
742+
}));
743+
if let Ok(b) = bytes {
744+
frame_cache()
745+
.lock()
746+
.unwrap_or_else(|e| e.into_inner())
747+
.insert(key, b, Some(1), None, current, key.scale_pct);
748+
}
749+
}
750+
SERVED.fetch_add(1, Ordering::Relaxed);
751+
// The canvas hit-map layout pass runs on every tick in the app.
752+
let _ = frame_hits(&s2, &t2, current, "/scenes/0");
753+
}
754+
});
755+
756+
while !sim.is_finished() {
757+
std::thread::sleep(Duration::from_secs(5));
758+
println!(
759+
"rss={} MB served={} misses={}",
760+
rss_mb(),
761+
SERVED.load(Ordering::Relaxed),
762+
MISSES.load(Ordering::Relaxed)
763+
);
764+
}
765+
sim.join().unwrap();
766+
println!(
767+
"end rss={} MB misses={}",
768+
rss_mb(),
769+
MISSES.load(Ordering::Relaxed)
770+
);
771+
}
772+
773+
#[test]
774+
fn fail_ledger_exhausts_after_max_attempts() {
775+
let mut ledger = FailLedger::default();
776+
let k = key(1, DiffSide::B, 7);
777+
assert!(!ledger.exhausted(&k));
778+
ledger.record_failure(k);
779+
assert!(!ledger.exhausted(&k), "one transient failure gets a retry");
780+
ledger.record_failure(k);
781+
assert!(ledger.exhausted(&k), "gives up after MAX_RENDER_ATTEMPTS");
782+
assert!(
783+
!ledger.exhausted(&key(2, DiffSide::B, 7)),
784+
"a new generation retries the same frame"
785+
);
786+
}
787+
626788
#[test]
627789
fn worker_count_leaves_ui_cores_and_stays_bounded() {
628790
assert_eq!(worker_count(1), 2, "floor: two workers even on tiny CPUs");

crates/rustmotion-studio/src/editor/view.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use super::playback::{
1717
playback_action, use_hot_reload, use_playback_clock, PlaybackAction, PlaybackBar,
1818
};
1919
use super::prefetch::{
20-
frame_cache, preview_scale_pct, scale_factor, use_prefetch_publisher, FrameKey,
20+
fail_ledger, frame_cache, preview_scale_pct, scale_factor, use_prefetch_publisher, FrameKey,
2121
};
2222
use super::topbar::TopBar;
2323

@@ -371,10 +371,25 @@ fn serve_or_render(
371371
{
372372
return Ok((*bytes).clone());
373373
}
374+
// A frame whose render keeps panicking is not retried on every <img>
375+
// request — fail fast so the webview shows the previous frame instead of
376+
// hammering a panic loop.
377+
if fail_ledger()
378+
.lock()
379+
.unwrap_or_else(|e| e.into_inner())
380+
.exhausted(&key)
381+
{
382+
return Err(());
383+
}
374384
let rendered = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
375385
render_frame(scenario, tasks, idx, scale_factor(key.scale_pct))
376386
}))
377-
.map_err(|_| ())?;
387+
.map_err(|_| {
388+
fail_ledger()
389+
.lock()
390+
.unwrap_or_else(|e| e.into_inner())
391+
.record_failure(key);
392+
})?;
378393
let (gen_b, gen_a) = match key.side {
379394
DiffSide::B => (gen_b, None),
380395
DiffSide::A => (None, Some(key.generation)),

0 commit comments

Comments
 (0)