From 492a781d92ae287d037249d7277515c29d290981 Mon Sep 17 00:00:00 2001 From: nevaforget Date: Tue, 2 Jun 2026 16:32:29 +0200 Subject: [PATCH] fix: prune per-monitor windows on monitor removal (v0.6.15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resume-unlock SIGSEGV: connect_monitor only adds windows to all_handles, never removes them when a monitor powers off on suspend. gtk4-session-lock unmaps and drops its own ref, but the GtkApplication and all_handles keep theirs, so the orphaned window survives until unlock — where destroying it dereferences its now-NULL monitor association and crashes. Watch display.monitors() and prune handles whose monitor is no longer valid, releasing the app ref via remove_window (the lib already unmapped and dereffed the window — we must not destroy it ourselves). Revert the earlier idle_add_local_once deferral: the logs proved it ineffective (crash happens inside the idle trampoline). Diagnostic unlock logging kept until a suspend/resume cycle confirms the fix. --- CLAUDE.md | 2 +- Cargo.lock | 2 +- Cargo.toml | 2 +- DECISIONS.md | 7 ++++++ src/lockscreen.rs | 5 +++++ src/main.rs | 55 +++++++++++++++++++++++++++++++++++++++++------ 6 files changed, 64 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1d17f4b..28e15c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,7 @@ LD_PRELOAD=/usr/lib/libgtk4-layer-shell.so ./target/release/moonlock - `i18n.rs` — Locale-Erkennung (OnceLock-cached) und String-Tabellen (DE/EN), faillock_warning mit konfigurierbarem max_attempts - `config.rs` — TOML-Config (background_path, background_blur clamped [0,200], fingerprint_enabled als Option) + Wallpaper-Fallback + Symlink-Rejection via symlink_metadata + Parse-Error-Logging - `lockscreen.rs` — GTK4 UI via LockscreenHandles, PAM-Auth via gio::spawn_blocking mit 30s Timeout und Generation Counter, FP-Success ruft unlock_callback direkt (PAM-Stack ohne account-Modul, Lockout via auth-Pfad und MAX_FP_ATTEMPTS), Zeroizing für Passwort, Power-Confirm, GPU-Blur via GskBlurNode (Downscale auf max 1920px), Blur/Avatar-Cache für Multi-Monitor -- `main.rs` — Entry Point, Panic-Hook (vor Logging), Root-Check, ext-session-lock-v1 (Pflicht in Release), Monitor-Hotplug via `connect_monitor`-Signal (v1_2), shared Blur/Avatar-Caches in Rc, systemd-Journal-Logging, Debug-Level per `MOONLOCK_DEBUG` Env-Var, async fprintd-Init nach window.present() +- `main.rs` — Entry Point, Panic-Hook (vor Logging), Root-Check, ext-session-lock-v1 (Pflicht in Release), Monitor-Hotplug via `connect_monitor`-Signal (v1_2) + Pruning toter Fenster bei Monitor-Removal (`display.monitors()` items_changed → `remove_window`) gegen Resume-Unlock-SIGSEGV, shared Blur/Avatar-Caches in Rc, systemd-Journal-Logging, Debug-Level per `MOONLOCK_DEBUG` Env-Var, async fprintd-Init nach window.present() ## Sicherheit diff --git a/Cargo.lock b/Cargo.lock index 01589f6..d1261d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,7 +575,7 @@ dependencies = [ [[package]] name = "moonlock" -version = "0.6.14" +version = "0.6.15" dependencies = [ "gdk-pixbuf", "gdk4", diff --git a/Cargo.toml b/Cargo.toml index e5e719d..7fcd026 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "moonlock" -version = "0.6.14" +version = "0.6.15" edition = "2024" description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support" license = "MIT" diff --git a/DECISIONS.md b/DECISIONS.md index 98d821d..c9ed8b4 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2,6 +2,13 @@ Architectural and design decisions for Moonlock, in reverse chronological order. +## 2026-06-02 – Prune per-monitor windows on monitor removal to fix resume-unlock SIGSEGV (v0.6.15) + +- **Who**: ClaudeCode, Dom +- **Why**: Chronic SIGSEGV (multiple coredumps/day) on the unlock following a suspend/resume. Backtrace: `app.quit()` → `gtk_window_destroy` → gtk4-layer-shell → `g_signal_emit` → NULL deref (`0x278`, rax=0). Root cause: `connect_monitor` is add-only — it creates one window per monitor and pushes to `all_handles`, but nothing ever removes a window when a monitor powers off on suspend. gtk4-session-lock unmaps + drops *its* ref to that window on monitor removal (per `gtk4-session-lock` 0.4 docs), but the GtkApplication (`ApplicationWindow::builder().application(app)`) and `all_handles` still hold refs. The orphaned window survives until unlock, where destroying it dereferences its now-NULL monitor association. A diagnostic confirmed the windows are GTK-valid but accumulate (3 windows for 1-2 monitors) with a NULL associated object. +- **Tradeoffs**: An earlier attempt deferred `unlock()`/`quit()` via `glib::idle_add_local_once` on a reentrancy theory — proven wrong by the logs (crash happens *inside* the idle trampoline). Reverted. The library doc explicitly says monitor removal is detected via "GTK APIs"; we follow that rather than fighting the library. We release our refs (do **not** call `destroy` — the lib already unmapped+dereffed the window). Diagnostic UNLOCK logging is kept for now, to be removed once a suspend/resume validation cycle confirms the fix. +- **How**: `LockscreenHandles` gains `monitor: Option`, set in the `connect_monitor` handler. `activate_with_session_lock` watches `display.monitors()` via `connect_items_changed`; on any change it retains only handles whose monitor `is_valid()`, calling `app.remove_window()` on the pruned ones to drop the application's ref. With both refs released, the orphaned window is gone before unlock. + ## 2026-06-02 – Align power-confirm to moonset's ActionDef pattern (v0.6.14) - **Who**: ClaudeCode, Dom diff --git a/src/lockscreen.rs b/src/lockscreen.rs index 214cc14..fdd505a 100644 --- a/src/lockscreen.rs +++ b/src/lockscreen.rs @@ -27,6 +27,9 @@ pub struct LockscreenHandles { pub password_entry: gtk::PasswordEntry, pub unlock_callback: Rc, pub username: String, + /// The monitor this window was assigned to (session-lock path only). + /// Used to prune the window when its monitor is removed on suspend/resume. + pub monitor: Option, state: Rc>, } @@ -70,6 +73,7 @@ pub fn create_lockscreen_window( password_entry: gtk::PasswordEntry::new(), unlock_callback, username: String::new(), + monitor: None, state: Rc::new(RefCell::new(LockscreenState { failed_attempts: 0, fp_listener_rc: None, @@ -360,6 +364,7 @@ pub fn create_lockscreen_window( password_entry: password_entry.clone(), unlock_callback, username: user.username, + monitor: None, state: state.clone(), } } diff --git a/src/main.rs b/src/main.rs index 602ecbf..389b68f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -59,7 +59,7 @@ fn activate(app: >k::Application) { fn activate_with_session_lock( app: >k::Application, - _display: &gdk::Display, + display: &gdk::Display, config: &config::Config, ) { let lock = gtk4_session_lock::Instance::new(); @@ -72,18 +72,39 @@ fn activate_with_session_lock( .and_then(|path| lockscreen::load_background_texture(&path)), ); + // Shared handles list — populated by connect_monitor, read by fingerprint init. + // Declared before unlock_callback so the callback can inspect window state on unlock. + let all_handles: Rc>> = + Rc::new(RefCell::new(Vec::new())); + // Shared unlock callback — unlocks session and quits. // Guard prevents double-unlock if PAM and fingerprint succeed simultaneously. let lock_clone = lock.clone(); let app_clone = app.clone(); let already_unlocked = Rc::new(Cell::new(false)); let au = already_unlocked.clone(); + let all_handles_dbg = all_handles.clone(); let unlock_callback: Rc = Rc::new(move || { if au.get() { log::debug!("Unlock already triggered, ignoring duplicate"); return; } au.set(true); + // DIAGNOSTIC: log lock-window state at unlock to confirm whether a stale + // window (unrealized / dead surface) remains in all_handles after a monitor + // power-off/resume. Remove once the resume-unlock crash is fixed. + { + let handles = all_handles_dbg.borrow(); + log::info!("UNLOCK: {} window(s) in all_handles", handles.len()); + for (i, h) in handles.iter().enumerate() { + log::info!( + "UNLOCK: window[{i}] realized={} mapped={} visible={}", + h.window.is_realized(), + h.window.is_mapped(), + h.window.is_visible(), + ); + } + } lock_clone.unlock(); app_clone.quit(); }); @@ -95,10 +116,6 @@ fn activate_with_session_lock( // Shared config for use in the monitor signal handler let config = Rc::new(config.clone()); - // Shared handles list — populated by connect_monitor, read by fingerprint init - let all_handles: Rc>> = - Rc::new(RefCell::new(Vec::new())); - // Shared fingerprint listener — None until async init completes. // The monitor handler checks this to wire up FP labels on hotplugged monitors. let shared_fp: Rc>>>> = @@ -127,7 +144,7 @@ fn activate_with_session_lock( shared_fp, move |_instance, monitor| { log::debug!("Monitor signal: creating lockscreen window"); - let handles = lockscreen::create_lockscreen_window( + let mut handles = lockscreen::create_lockscreen_window( bg_texture.as_ref().as_ref(), &config, &app, @@ -137,6 +154,7 @@ fn activate_with_session_lock( ); lock_for_signal.assign_window_to_monitor(&handles.window, monitor); handles.window.present(); + handles.monitor = Some(monitor.clone()); // If fingerprint is already initialized, wire up the label if let Some(ref fp_rc) = *shared_fp.borrow() { @@ -147,6 +165,31 @@ fn activate_with_session_lock( } )); + // connect_monitor only ADDS — it never tells us when a monitor powers off on + // suspend. gtk4-session-lock then unmaps and drops its own ref to that monitor's + // window, but the GtkApplication and all_handles still hold refs, so the orphaned + // window survives until unlock — where gtk_window_destroy dereferences its now-NULL + // monitor association and segfaults. Watch the display's monitor list and prune any + // window whose monitor is no longer valid, releasing our refs (the lib doc points to + // "GTK APIs" for exactly this). We release refs, not destroy — the lib already + // unmapped+dereffed the window. + display.monitors().connect_items_changed(glib::clone!( + #[weak] + app, + #[strong] + all_handles, + move |_, _, _, _| { + all_handles.borrow_mut().retain(|h| { + let alive = h.monitor.as_ref().is_none_or(gdk::Monitor::is_valid); + if !alive { + log::info!("Monitor removed — pruning its lockscreen window"); + app.remove_window(&h.window); + } + alive + }); + } + )); + lock.lock(); // Async fprintd initialization — runs after windows are visible