fix: audit fixes — RefCell across await, async avatar decode (v0.6.10)
- init_fingerprint_async: hoist username before the await so a concurrent connect_monitor signal (hotplug / suspend-resume) cannot cause a RefCell panic. Re-borrow after the await for signal wiring. - set_avatar_from_file: decode via gio::File::read_future + Pixbuf::from_stream_at_scale_future so the GTK main thread stays responsive during monitor hotplug. Default icon shown while loading.
This commit is contained in:
parent
3adc5e980d
commit
39d9cbb624
2
Cargo.lock
generated
2
Cargo.lock
generated
@ -575,7 +575,7 @@ dependencies = [
|
|||||||
|
|
||||||
[[package]]
|
[[package]]
|
||||||
name = "moonlock"
|
name = "moonlock"
|
||||||
version = "0.6.8"
|
version = "0.6.9"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"gdk-pixbuf",
|
"gdk-pixbuf",
|
||||||
"gdk4",
|
"gdk4",
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "moonlock"
|
name = "moonlock"
|
||||||
version = "0.6.9"
|
version = "0.6.10"
|
||||||
edition = "2024"
|
edition = "2024"
|
||||||
description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support"
|
description = "A secure Wayland lockscreen with GTK4, PAM and fingerprint support"
|
||||||
license = "MIT"
|
license = "MIT"
|
||||||
|
|||||||
@ -2,6 +2,13 @@
|
|||||||
|
|
||||||
Architectural and design decisions for Moonlock, in reverse chronological order.
|
Architectural and design decisions for Moonlock, in reverse chronological order.
|
||||||
|
|
||||||
|
## 2026-04-24 – Audit fixes: RefCell borrow across await, async avatar decode
|
||||||
|
|
||||||
|
- **Who**: ClaudeCode, Dom
|
||||||
|
- **Why**: Triple audit found two HIGH issues. (1) `init_fingerprint_async` held a `RefCell` immutable borrow across `is_available_async().await` — a concurrent `connect_monitor` signal (hotplug / suspend-resume) invoking `borrow_mut()` during the await would panic. (2) `set_avatar_from_file` decoded avatars synchronously via `Pixbuf::from_file_at_scale`, blocking the GTK main thread inside the `connect_monitor` handler. With `MAX_AVATAR_FILE_SIZE` at 10 MB the worst-case stall was 200–500 ms on monitor hotplug.
|
||||||
|
- **Tradeoffs**: Avatar is shown as the symbolic default icon for a brief window while decoding completes. Wallpaper stays synchronous because `connect_monitor` fires during `lock()` and needs the texture already present (see 2026-04-09).
|
||||||
|
- **How**: (1) Extract `username` into a local `String` in `init_fingerprint_async`, drop the borrow before the await, re-borrow in a new scope after — no awaits inside the second borrow, so hotplug during signal setup is safe. (2) `set_avatar_from_file` now uses `gio::File::read_future` + `Pixbuf::from_stream_at_scale_future` for async I/O and decode. The default icon is shown immediately; the decoded texture replaces it when ready. `Pixbuf` itself is `!Send`, so `gio::spawn_blocking` does not apply — the GIO async stream loader keeps the `Pixbuf` on the main thread while the kernel does the I/O asynchronously.
|
||||||
|
|
||||||
## 2026-04-09 – Monitor hotplug via connect_monitor signal
|
## 2026-04-09 – Monitor hotplug via connect_monitor signal
|
||||||
|
|
||||||
- **Who**: ClaudeCode, Dom
|
- **Who**: ClaudeCode, Dom
|
||||||
|
|||||||
@ -623,30 +623,41 @@ fn render_blurred_texture(
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Load an image file and set it as the avatar. Stores the texture in the cache.
|
/// Load an image file and set it as the avatar. Stores the texture in the cache.
|
||||||
|
/// Decoding runs via GIO async I/O + async pixbuf stream loader so the GTK main
|
||||||
|
/// loop stays responsive — avatars may be loaded inside the `connect_monitor`
|
||||||
|
/// signal handler at hotplug time, which must not block. The fallback icon is
|
||||||
|
/// shown immediately; the decoded texture replaces it when ready.
|
||||||
fn set_avatar_from_file(
|
fn set_avatar_from_file(
|
||||||
image: >k::Image,
|
image: >k::Image,
|
||||||
path: &Path,
|
path: &Path,
|
||||||
cache: &Rc<RefCell<Option<gdk::Texture>>>,
|
cache: &Rc<RefCell<Option<gdk::Texture>>>,
|
||||||
) {
|
) {
|
||||||
let path_str = match path.to_str() {
|
|
||||||
Some(s) => s,
|
|
||||||
None => {
|
|
||||||
log::warn!("Avatar path is not valid UTF-8: {:?}", path);
|
|
||||||
image.set_icon_name(Some("avatar-default-symbolic"));
|
image.set_icon_name(Some("avatar-default-symbolic"));
|
||||||
|
|
||||||
|
let display_path = path.to_path_buf();
|
||||||
|
let file = gio::File::for_path(path);
|
||||||
|
let image_clone = image.clone();
|
||||||
|
let cache_clone = cache.clone();
|
||||||
|
|
||||||
|
glib::spawn_future_local(async move {
|
||||||
|
let stream = match file.read_future(glib::Priority::default()).await {
|
||||||
|
Ok(s) => s,
|
||||||
|
Err(e) => {
|
||||||
|
log::warn!("Failed to open avatar {}: {e}", display_path.display());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
match Pixbuf::from_file_at_scale(path_str, AVATAR_SIZE, AVATAR_SIZE, true) {
|
match Pixbuf::from_stream_at_scale_future(&stream, AVATAR_SIZE, AVATAR_SIZE, true).await {
|
||||||
Ok(pixbuf) => {
|
Ok(pixbuf) => {
|
||||||
let texture = gdk::Texture::for_pixbuf(&pixbuf);
|
let texture = gdk::Texture::for_pixbuf(&pixbuf);
|
||||||
image.set_paintable(Some(&texture));
|
image_clone.set_paintable(Some(&texture));
|
||||||
*cache.borrow_mut() = Some(texture);
|
*cache_clone.borrow_mut() = Some(texture);
|
||||||
}
|
}
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
log::warn!("Failed to load avatar from {:?}: {e}", path);
|
log::warn!("Failed to decode avatar from {}: {e}", display_path.display());
|
||||||
image.set_icon_name(Some("avatar-default-symbolic"));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Load the default avatar SVG from GResources, tinted with the foreground color.
|
/// Load the default avatar SVG from GResources, tinted with the foreground color.
|
||||||
|
|||||||
22
src/main.rs
22
src/main.rs
@ -168,31 +168,37 @@ fn init_fingerprint_async(
|
|||||||
let mut listener = FingerprintListener::new();
|
let mut listener = FingerprintListener::new();
|
||||||
listener.init_async().await;
|
listener.init_async().await;
|
||||||
|
|
||||||
|
// Extract username without holding a borrow across the await below —
|
||||||
|
// otherwise a concurrent connect_monitor signal (hotplug / suspend-resume)
|
||||||
|
// that tries to borrow_mut() panics at runtime.
|
||||||
|
let username = {
|
||||||
let handles = all_handles.borrow();
|
let handles = all_handles.borrow();
|
||||||
if handles.is_empty() {
|
if handles.is_empty() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
let u = handles[0].username.clone();
|
||||||
// Use the first monitor's username to check enrollment
|
if u.is_empty() {
|
||||||
let username = &handles[0].username;
|
|
||||||
if username.is_empty() {
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
u
|
||||||
|
};
|
||||||
|
|
||||||
if !listener.is_available_async(username).await {
|
if !listener.is_available_async(&username).await {
|
||||||
log::debug!("fprintd not available or no enrolled fingers");
|
log::debug!("fprintd not available or no enrolled fingers");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let fp_rc = Rc::new(RefCell::new(listener));
|
let fp_rc = Rc::new(RefCell::new(listener));
|
||||||
|
|
||||||
// Show fingerprint label on all existing monitors
|
// Re-borrow after the await — no further awaits in this scope, so it is
|
||||||
|
// safe to hold the borrow briefly while wiring up the labels.
|
||||||
|
{
|
||||||
|
let handles = all_handles.borrow();
|
||||||
for h in handles.iter() {
|
for h in handles.iter() {
|
||||||
lockscreen::show_fingerprint_label(h, &fp_rc);
|
lockscreen::show_fingerprint_label(h, &fp_rc);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Start verification listener on the first monitor only
|
|
||||||
lockscreen::start_fingerprint(&handles[0], &fp_rc);
|
lockscreen::start_fingerprint(&handles[0], &fp_rc);
|
||||||
|
}
|
||||||
|
|
||||||
// Publish the listener so hotplugged monitors get FP labels too
|
// Publish the listener so hotplugged monitors get FP labels too
|
||||||
*shared_fp.borrow_mut() = Some(fp_rc);
|
*shared_fp.borrow_mut() = Some(fp_rc);
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user