fix-deviant(core): bound app mailbox work per tick
A freeze report from continuous newline-free base64 output exposed an unbounded mailbox drain: producers can keep refilling the queue and prevent one app tick from returning to the runtime for input, timers, and drawing. Process at most 64 messages per tick and wake the runtime if work remains. Messages retain their order and are not dropped. Document the scheduling contract and the deliberate deviation from the upstream drain loop. Add a finite refilling-producer regression that fails with the original loop and verifies yielding, wakeup rearming, and eventual completion. The reported full freeze was not consistently reproduced in headless GTK, so this fixes a demonstrated starvation risk without claiming its root cause. Test Plan: - All 119 ghostty-core tests pass with Mesa software OpenGL. - Release build, Clippy for all core targets with -D warnings, and fmt pass. - GTK handles Ctrl+C, a follow-up command, and quit after a 15-second flood. Reference: docs/rewrite/DEVIANT_FIXES.md (entry 40)
This commit is contained in:
1 parent
85489a164e
commit
4c31d27888
3 files changed
+80
-2
No files matched your search
@@ -440,9 +440,14 @@ impl App {
|
||||
rt.perform_action(target, RtAction::NewWindow);
|
||||
}
|
||||
|
||||
/// Drain the mailbox.
|
||||
/// Process at most one mailbox capacity per tick, then return to the
|
||||
/// runtime so it can dispatch input, timers, and drawing. Producers can
|
||||
/// refill the queue continuously under heavy terminal output.
|
||||
fn drain_mailbox(&self, rt: &dyn RtApp) {
|
||||
while let Ok(message) = self.state.mailbox_rx.try_recv() {
|
||||
for _ in 0..MAILBOX_CAPACITY {
|
||||
let Ok(message) = self.state.mailbox_rx.try_recv() else {
|
||||
return;
|
||||
};
|
||||
match message {
|
||||
Message::OpenConfig => {
|
||||
let _ = self.perform_action(rt, &BindingAction::OpenConfig);
|
||||
@@ -469,6 +474,12 @@ impl App {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Producer wakeups may have coalesced during this tick. Ensure a
|
||||
// fresh iteration for deferred messages even if producers stop now.
|
||||
if !self.state.mailbox_rx.is_empty() {
|
||||
self.state.mailbox.waker.wake();
|
||||
}
|
||||
}
|
||||
|
||||
/// Close a surface (Zig: `closeSurface`).
|
||||
@@ -657,3 +668,65 @@ impl CoreApp for App {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
|
||||
use super::*;
|
||||
|
||||
#[derive(Default)]
|
||||
struct CountingWaker(AtomicUsize);
|
||||
|
||||
impl Waker for CountingWaker {
|
||||
fn wake(&self) {
|
||||
self.0.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
}
|
||||
|
||||
/// Refill the mailbox during each callback, like a renderer producing
|
||||
/// redraw requests while the app processes earlier requests.
|
||||
struct RefillingRuntime {
|
||||
mailbox: Mailbox,
|
||||
calls: Cell<usize>,
|
||||
}
|
||||
|
||||
impl RtApp for RefillingRuntime {
|
||||
fn perform_action(&self, _: Target<SurfaceId>, action: RtAction) -> bool {
|
||||
assert!(matches!(action, RtAction::NewWindow));
|
||||
let calls = self.calls.get() + 1;
|
||||
self.calls.set(calls);
|
||||
// Finite even with an unbounded drain, so regressions fail
|
||||
// without hanging the test runner.
|
||||
if calls < MAILBOX_CAPACITY * 2 {
|
||||
self.mailbox
|
||||
.push(Message::NewWindow { parent: None }, Timeout::Instant);
|
||||
}
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mailbox_tick_yields_and_rearms_when_producer_keeps_refilling() {
|
||||
let waker = Arc::new(CountingWaker::default());
|
||||
let app = App::new(waker.clone()).unwrap();
|
||||
let rt = RefillingRuntime {
|
||||
mailbox: app.mailbox(),
|
||||
calls: Cell::new(0),
|
||||
};
|
||||
rt.mailbox
|
||||
.push(Message::NewWindow { parent: None }, Timeout::Instant);
|
||||
|
||||
app.tick(&rt).unwrap();
|
||||
assert_eq!(rt.calls.get(), MAILBOX_CAPACITY);
|
||||
assert_eq!(app.state.mailbox_rx.len(), 1);
|
||||
// Initial push, one push per callback, and a wake to resume the
|
||||
// pending work after the runtime has handled input and timers.
|
||||
assert_eq!(waker.0.load(Ordering::Relaxed), MAILBOX_CAPACITY + 2);
|
||||
|
||||
app.tick(&rt).unwrap();
|
||||
assert_eq!(rt.calls.get(), MAILBOX_CAPACITY * 2);
|
||||
assert!(app.state.mailbox_rx.is_empty());
|
||||
assert_eq!(waker.0.load(Ordering::Relaxed), MAILBOX_CAPACITY * 2 + 1);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user