fix(ui): the push-pull gizmo re-entered egui's context lock and froze the app (e4dd7ddb)
FOUND BY PETI IN ONE CLICK on a release build: click the part, the push-pull window opens,
the app is dead. Diagnosed from a core dump of his own frozen process (SIGABRT on pid
2585246, which was still parked), so the mechanism is read off the real failure rather than
inferred.
THE STACK NAMES IT, and it is one thread, not a cycle:
parking_lot::raw_rwlock::RawRwLock::lock_exclusive_slow
egui::context::Context::write
vernier_ui::shell::gizmo_overlay
<VernierApp as ApplicationHandler>::window_event::{{closure}}
egui::context::Context::run_ui_dyn
gizmo_overlay called `response.drag_delta()` INSIDE a `ui.data_mut` closure. In egui 0.36
`Context::data_mut` and `Context::input` both go through `Context::write`, the same exclusive
lock, and `Response::drag_delta` calls `ctx.input`. parking_lot's RwLock is not reentrant, so
the UI thread asked itself for a lock it already held and parked on a futex nothing could
wake.
WHY IT LOOKED LIKE A CROSS-THREAD LOCK CYCLE: all 16 threads sat in futex_wait with zero CPU.
Fourteen of them are idle RADV/gallium worker pools and the WSI swapchain pair, and the
fifteenth is our own document-server parked in mpsc::recv. Only the main thread was stuck on
anything. An idle app looks the same in wchan; the backtrace is what separates them.
WHY ONE CLICK WAS ENOUGH: the branch needs `response.dragged()`, and the click that opens the
window leaves the button down over the handle, so the very next frame reports a drag.
WHY ONLY THE RELEASE BUILD DIES: epaint's RwLock is `try_write_for(10s)` + panic under
debug_assertions and a plain blocking `write()` without them. Debug builds get "DEBUG PANIC:
Failed to acquire RwLock write after 10s. Deadlock?"; release builds block forever.
THE FIX is to read the delta before taking the lock. One hoist, no behaviour change.
THE TEST is a watchdog, because a deadlock has no return value: it drives four synthesized
frames (the handle must exist for a pass before a press on it can land, and the press before a
move counts as a drag) on their own thread and fails if they do not finish. It also asserts the
drag actually happened, so a run that never reached the branch cannot pass for the wrong
reason -- that guard fired for real on the first attempt, when three frames were not enough.
DRIVEN BOTH WAYS: with `drag_delta()` back inside the closure the test fails in 10.0s; with the
hoist it passes in 0.01s. Workspace green, clippy exit 0.
SWEPT FOR THE SAME CLASS: nine egui context-lock closures in vernier-ui and vernier-app. This
was the only one that called back into the context from inside one; the other eight touch only
their closure argument.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0191VzW8Ykq4aWDJqdwQRcsi
This commit is contained in:
@@ -4935,8 +4935,16 @@ pub fn gizmo_overlay(ui: &mut egui::Ui, anchor_px: [f32; 2], tip_px: [f32; 2]) -
|
||||
ui.data_mut(|data| data.insert_temp(id, egui::Vec2::ZERO));
|
||||
}
|
||||
if response.dragged() || response.drag_stopped() {
|
||||
// READ THE DELTA BEFORE TAKING THE LOCK, and keep it out here.
|
||||
// `ui.data_mut` holds egui's ONE exclusive Context lock for the whole
|
||||
// closure, and `Response::drag_delta` reaches back into the same lock
|
||||
// through `ctx.input`. parking_lot's RwLock is not reentrant, so asking
|
||||
// for it twice on one thread parks that thread forever -- in a release
|
||||
// build silently, since epaint's 10-second deadlock panic is
|
||||
// debug-only. That is card e4dd7ddb: one click, whole app gone.
|
||||
let delta = response.drag_delta();
|
||||
let total = ui.data_mut(|data| {
|
||||
let total = data.get_temp::<egui::Vec2>(id).unwrap_or_default() + response.drag_delta();
|
||||
let total = data.get_temp::<egui::Vec2>(id).unwrap_or_default() + delta;
|
||||
data.insert_temp(id, total);
|
||||
total
|
||||
});
|
||||
@@ -5025,6 +5033,99 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// **THE PUSH-PULL GIZMO DEADLOCKED THE APP ON THE FIRST DRAG** (card
|
||||
/// e4dd7ddb, found by Peti in one click on a release build: click the part,
|
||||
/// the push-pull window opens, the app is dead from that instant).
|
||||
///
|
||||
/// In egui 0.36 `Context::input` takes the SAME exclusive lock as
|
||||
/// `Context::data_mut` — both go through `Context::write` — and
|
||||
/// parking_lot's `RwLock` is not reentrant. `Response::drag_delta` calls
|
||||
/// `ctx.input`, so calling it INSIDE a `ui.data_mut` closure parks the UI
|
||||
/// thread on a futex only that same thread could release. The process then
|
||||
/// shows every thread in `futex_wait` and zero CPU, which reads like a
|
||||
/// cross-thread lock cycle and is not one: the other fifteen are idle
|
||||
/// driver pools and an idle worker.
|
||||
///
|
||||
/// IT NEEDS A DRAG, which is why one click was enough — the click that
|
||||
/// opens the window leaves the button down over the handle, so the next
|
||||
/// frame reports `dragged()` and takes the branch.
|
||||
///
|
||||
/// A WATCHDOG RATHER THAN AN ASSERTION ON A VALUE, because a deadlock
|
||||
/// returns nothing to assert on. It runs the frames on their own thread and
|
||||
/// fails by timeout, which is the only shape this defect has. Put
|
||||
/// `response.drag_delta()` back inside the closure and this test stops
|
||||
/// finishing.
|
||||
#[test]
|
||||
fn dragging_the_gizmo_does_not_deadlock_the_ui_thread() {
|
||||
let (fertig_tx, fertig_rx) = std::sync::mpsc::channel();
|
||||
std::thread::spawn(move || {
|
||||
let ctx = shell_ctx();
|
||||
let (anchor, tip) = ([100.0_f32, 100.0], [200.0_f32, 100.0]);
|
||||
// anchor + normalized(tip - anchor) * 60.0: the handle the overlay
|
||||
// paints and interacts on.
|
||||
let handle = egui::pos2(160.0, 100.0);
|
||||
let id = egui::Id::new("vernier-pushpull-gizmo");
|
||||
let press = egui::Event::PointerButton {
|
||||
pos: handle,
|
||||
button: egui::PointerButton::Primary,
|
||||
pressed: true,
|
||||
modifiers: egui::Modifiers::default(),
|
||||
};
|
||||
// FOUR FRAMES, because egui resolves interaction against the
|
||||
// widget rects of the PREVIOUS pass: the handle has to exist for a
|
||||
// frame before a press on it can land, and the press has to land
|
||||
// before a move counts as a drag.
|
||||
let frames = [
|
||||
vec![egui::Event::PointerMoved(handle)],
|
||||
vec![egui::Event::PointerMoved(handle), press],
|
||||
vec![egui::Event::PointerMoved(handle + egui::vec2(20.0, 0.0))],
|
||||
vec![egui::Event::PointerMoved(handle + egui::vec2(40.0, 0.0))],
|
||||
];
|
||||
let mut gezogen = false;
|
||||
for events in frames {
|
||||
let input = egui::RawInput {
|
||||
events,
|
||||
..frame_input()
|
||||
};
|
||||
let output = ctx.run_ui(input, |ui| {
|
||||
// Read the drag state BEFORE the overlay runs: this is the
|
||||
// state its own `response.dragged()` will report, and it is
|
||||
// the last thing observable if the call never returns.
|
||||
gezogen |= ui.ctx().is_being_dragged(id);
|
||||
let _ = gizmo_overlay(ui, anchor, tip);
|
||||
});
|
||||
output.drop_without_applying_deltas();
|
||||
}
|
||||
let _ = fertig_tx.send(gezogen);
|
||||
});
|
||||
match fertig_rx.recv_timeout(std::time::Duration::from_secs(30)) {
|
||||
// Separated on purpose: a panicking thread drops the sender and
|
||||
// arrives here too, and reporting that as a deadlock would be an
|
||||
// invented diagnosis.
|
||||
Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => panic!(
|
||||
"THE UI THREAD RE-ENTERED THE egui CONTEXT LOCK. The frame thread panicked \
|
||||
rather than hanging because epaint's RwLock only detects this in DEBUG builds \
|
||||
(\"DEBUG PANIC: Failed to acquire RwLock write after 10s\") -- its panic is \
|
||||
above. In a release build that same lock call blocks forever, which is how \
|
||||
card e4dd7ddb reached Peti: one click and the app is gone."
|
||||
),
|
||||
Err(std::sync::mpsc::RecvTimeoutError::Timeout) => panic!(
|
||||
"THE UI THREAD DEADLOCKED. gizmo_overlay took the exclusive egui Context lock \
|
||||
and then asked for it again on the same thread, so the drag branch never \
|
||||
returns. This is the defect from card e4dd7ddb: one click and the whole app is \
|
||||
gone, with every thread in futex_wait and no CPU burned."
|
||||
),
|
||||
// A green run that never dragged would pin nothing: the deadlock
|
||||
// lives inside the drag branch and nowhere else.
|
||||
Ok(gezogen) => assert!(
|
||||
gezogen,
|
||||
"the gizmo was never dragged, so this test would stay green with the deadlock \
|
||||
restored. The synthesized press-then-move no longer registers a drag on the \
|
||||
handle."
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Two frames, and the text the SECOND one painted.
|
||||
///
|
||||
/// **AN `egui::Area` PAINTS NOTHING ON ITS FIRST PASS** — it has no size
|
||||
|
||||
Reference in New Issue
Block a user