From 1ef74a787d77a13eac777cecd10b72c9ee548952 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 5 Oct 2026 02:06:08 -0400 Subject: [PATCH] Never crash: move the one unsafe FFI call into crates/sysmem The macOS malloc_zone_pressure_relief call (returns freed allocator pages after raw decodes) lived in identical copies inside the desktop app and the CLI. It now lives in lightcraft-sysmem, an isolated L0 helper crate and the workspace's only crate allowed unsafe: one #[allow(unsafe_code)] module, a SAFETY comment, a safe API that returns bytes released (0 off macOS), and tests (repeat calls, release after large frees, concurrent calls). Both apps' alloc_release.rs now just install the safe hook, and their crate roots gain #![forbid(unsafe_code)], so every production crate except sysmem forbids it. AGENTS.md/CLAUDE.md name the exception, per craftrules standards/never-crash.md. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 ++ CLAUDE.md | 4 ++ Cargo.lock | 6 ++ Cargo.toml | 1 + apps/lightcraft-cli/Cargo.toml | 1 + apps/lightcraft-cli/src/alloc_release.rs | 32 ++-------- apps/lightcraft-cli/src/main.rs | 1 + apps/lightcraft/Cargo.toml | 1 + apps/lightcraft/src/alloc_release.rs | 32 ++-------- apps/lightcraft/src/main.rs | 1 + crates/sysmem/Cargo.toml | 18 ++++++ crates/sysmem/src/lib.rs | 80 ++++++++++++++++++++++++ xtask/src/layers.rs | 1 + 13 files changed, 130 insertions(+), 52 deletions(-) create mode 100644 crates/sysmem/Cargo.toml create mode 100644 crates/sysmem/src/lib.rs diff --git a/AGENTS.md b/AGENTS.md index 190c759b..4aaddb3b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,6 +28,10 @@ of it. Full standard: `../craftrules/standards/never-crash.md` fallback (`unwrap_or…`) only where it can't silently corrupt a document. An unfinished feature returns an "unsupported" error or is disabled. Sole exception: a provably infallible literal, as `#[allow(clippy::expect_used)]` + `.expect("why it can't fail")`. +- **`unsafe` lives only in `crates/sysmem`** (one FFI call, `malloc_zone_pressure_relief`, that returns freed + allocator pages to macOS after raw decodes). Every other production crate root has `#![forbid(unsafe_code)]`. A new + unsafe need goes in an isolated, well-tested helper crate like it: `// SAFETY:` on every block, a safe API, a safe + fallback where possible, and a line here naming it. - **Input-derived numbers are hostile:** `get()` instead of `[i]`/`[a..b]` for offsets from files, users, agents or arithmetic on them; checked/saturating math for lengths, offsets and counts; no division by zero, NaN/inf or negative casts to `usize`; cap allocations sized by input; slice strings only at char boundaries. diff --git a/CLAUDE.md b/CLAUDE.md index 190c759b..4aaddb3b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,10 @@ of it. Full standard: `../craftrules/standards/never-crash.md` fallback (`unwrap_or…`) only where it can't silently corrupt a document. An unfinished feature returns an "unsupported" error or is disabled. Sole exception: a provably infallible literal, as `#[allow(clippy::expect_used)]` + `.expect("why it can't fail")`. +- **`unsafe` lives only in `crates/sysmem`** (one FFI call, `malloc_zone_pressure_relief`, that returns freed + allocator pages to macOS after raw decodes). Every other production crate root has `#![forbid(unsafe_code)]`. A new + unsafe need goes in an isolated, well-tested helper crate like it: `// SAFETY:` on every block, a safe API, a safe + fallback where possible, and a line here naming it. - **Input-derived numbers are hostile:** `get()` instead of `[i]`/`[a..b]` for offsets from files, users, agents or arithmetic on them; checked/saturating math for lengths, offsets and counts; no division by zero, NaN/inf or negative casts to `usize`; cap allocations sized by input; slice strings only at char boundaries. diff --git a/Cargo.lock b/Cargo.lock index 60a82a26..a8fef8e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2080,6 +2080,7 @@ dependencies = [ "lightcraft-codecs", "lightcraft-engine", "lightcraft-raster", + "lightcraft-sysmem", "lightcraft-ui-egui", "log", "muda", @@ -2112,6 +2113,7 @@ dependencies = [ "lightcraft-mcp", "lightcraft-merge", "lightcraft-raster", + "lightcraft-sysmem", "lightcraft-ui-egui", "serde_json", ] @@ -2321,6 +2323,10 @@ dependencies = [ "serde", ] +[[package]] +name = "lightcraft-sysmem" +version = "0.1.1" + [[package]] name = "lightcraft-tiff" version = "0.1.1" diff --git a/Cargo.toml b/Cargo.toml index 832560a6..af8e8391 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ repository = "https://github.com/storytold/lightcraft" [workspace.dependencies] # internal lightcraft-geom = { path = "crates/geom" } +lightcraft-sysmem = { path = "crates/sysmem" } lightcraft-color = { path = "crates/color" } lightcraft-raster = { path = "crates/raster" } lightcraft-tiff = { path = "crates/tiff" } diff --git a/apps/lightcraft-cli/Cargo.toml b/apps/lightcraft-cli/Cargo.toml index 36c37c32..3f60fe32 100644 --- a/apps/lightcraft-cli/Cargo.toml +++ b/apps/lightcraft-cli/Cargo.toml @@ -15,6 +15,7 @@ workspace = true [dependencies] lightcraft-engine = { workspace = true } +lightcraft-sysmem = { workspace = true } lightcraft-mcp = { workspace = true } lightcraft-ui-egui = { workspace = true } lightcraft-codecs = { workspace = true } diff --git a/apps/lightcraft-cli/src/alloc_release.rs b/apps/lightcraft-cli/src/alloc_release.rs index 0d0775ba..83056aef 100644 --- a/apps/lightcraft-cli/src/alloc_release.rs +++ b/apps/lightcraft-cli/src/alloc_release.rs @@ -1,32 +1,12 @@ //! Give memory the system allocator keeps after frees back to the system -//! ([`lightcraft_engine::memory::set_release_hook`]). -//! -//! macOS's allocator keeps freed large blocks mapped as "reusable": they no longer count in the -//! process's memory footprint but stay in its resident size until the system runs short. After a -//! raw decode (hundreds of MB of temporaries) that made the resident size climb towards the sum of -//! everything ever decoded. `malloc_zone_pressure_relief` (libSystem) returns those pages now. -//! Elsewhere (glibc, Windows) blocks this large are unmapped when freed: nothing to do. +//! ([`lightcraft_engine::memory::set_release_hook`]). The platform call lives in +//! `lightcraft-sysmem`, the workspace's only crate allowed `unsafe`. -/// Install the hook for this platform. +/// Install the hook. pub fn install() { - #[cfg(target_os = "macos")] - lightcraft_engine::memory::set_release_hook(macos::release); + lightcraft_engine::memory::set_release_hook(release); } -#[cfg(target_os = "macos")] -#[allow(unsafe_code)] -mod macos { - unsafe extern "C" { - /// libSystem malloc: release free memory of `zone` (all zones when null) back to the - /// system, up to `goal` bytes (0 = as much as possible). Returns the bytes released. - fn malloc_zone_pressure_relief(zone: *mut std::ffi::c_void, goal: usize) -> usize; - } - - pub fn release() { - // SAFETY: a documented libSystem function taking a null zone (= every zone) and a size; - // it only returns free pages to the system and touches no memory we own. - unsafe { - malloc_zone_pressure_relief(std::ptr::null_mut(), 0); - } - } +fn release() { + let _ = lightcraft_sysmem::release_free_memory(); } diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index 1c2b7bf8..4f68ec88 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -10,6 +10,7 @@ //! lightcraft-cli commands [--json] //! lightcraft-cli controls [--json] //! ``` +#![forbid(unsafe_code)] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] mod alloc_release; diff --git a/apps/lightcraft/Cargo.toml b/apps/lightcraft/Cargo.toml index e81e79eb..46acbb73 100644 --- a/apps/lightcraft/Cargo.toml +++ b/apps/lightcraft/Cargo.toml @@ -11,6 +11,7 @@ workspace = true [dependencies] lightcraft-engine = { workspace = true } +lightcraft-sysmem = { workspace = true } lightcraft-ui-egui = { workspace = true } lightcraft-codecs = { workspace = true } lightcraft-raster = { workspace = true } diff --git a/apps/lightcraft/src/alloc_release.rs b/apps/lightcraft/src/alloc_release.rs index 0d0775ba..83056aef 100644 --- a/apps/lightcraft/src/alloc_release.rs +++ b/apps/lightcraft/src/alloc_release.rs @@ -1,32 +1,12 @@ //! Give memory the system allocator keeps after frees back to the system -//! ([`lightcraft_engine::memory::set_release_hook`]). -//! -//! macOS's allocator keeps freed large blocks mapped as "reusable": they no longer count in the -//! process's memory footprint but stay in its resident size until the system runs short. After a -//! raw decode (hundreds of MB of temporaries) that made the resident size climb towards the sum of -//! everything ever decoded. `malloc_zone_pressure_relief` (libSystem) returns those pages now. -//! Elsewhere (glibc, Windows) blocks this large are unmapped when freed: nothing to do. +//! ([`lightcraft_engine::memory::set_release_hook`]). The platform call lives in +//! `lightcraft-sysmem`, the workspace's only crate allowed `unsafe`. -/// Install the hook for this platform. +/// Install the hook. pub fn install() { - #[cfg(target_os = "macos")] - lightcraft_engine::memory::set_release_hook(macos::release); + lightcraft_engine::memory::set_release_hook(release); } -#[cfg(target_os = "macos")] -#[allow(unsafe_code)] -mod macos { - unsafe extern "C" { - /// libSystem malloc: release free memory of `zone` (all zones when null) back to the - /// system, up to `goal` bytes (0 = as much as possible). Returns the bytes released. - fn malloc_zone_pressure_relief(zone: *mut std::ffi::c_void, goal: usize) -> usize; - } - - pub fn release() { - // SAFETY: a documented libSystem function taking a null zone (= every zone) and a size; - // it only returns free pages to the system and touches no memory we own. - unsafe { - malloc_zone_pressure_relief(std::ptr::null_mut(), 0); - } - } +fn release() { + let _ = lightcraft_sysmem::release_free_memory(); } diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 9c8d63d1..981867c8 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -17,6 +17,7 @@ //! window (issue #7). Their `--help` / `--version` output and diagnostics then have no console to //! go to; use `lightcraft-cli` (a console program) from a terminal, or a debug build. #![cfg_attr(all(target_os = "windows", not(debug_assertions)), windows_subsystem = "windows")] +#![forbid(unsafe_code)] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] mod alloc_release; diff --git a/crates/sysmem/Cargo.toml b/crates/sysmem/Cargo.toml new file mode 100644 index 00000000..422d8c71 --- /dev/null +++ b/crates/sysmem/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "lightcraft-sysmem" +version.workspace = true +edition.workspace = true +license.workspace = true +description = "LightCraft sysmem (L0): returns freed allocator memory to the OS. The workspace's only crate allowed `unsafe` (one FFI call)." + +# Opts out of the workspace lints only to keep `unsafe_code = "deny"` overridable for the single FFI +# module below; every other crate root has `#![forbid(unsafe_code)]`. See AGENTS.md and craftrules +# standards/never-crash.md ("isolated helper library crate"). +[lints.rust] +unsafe_code = "deny" + +[lints.clippy] +dbg_macro = "warn" +todo = "warn" + +[dependencies] diff --git a/crates/sysmem/src/lib.rs b/crates/sysmem/src/lib.rs new file mode 100644 index 00000000..2961f537 --- /dev/null +++ b/crates/sysmem/src/lib.rs @@ -0,0 +1,80 @@ +//! Give memory the system allocator keeps after frees back to the operating system. +//! +//! macOS's allocator keeps freed large blocks mapped as "reusable": they no longer count in the +//! process's memory footprint but stay in its resident size until the system runs short. After a +//! raw decode (hundreds of MB of temporaries) that made the resident size climb towards the sum of +//! everything ever decoded. `malloc_zone_pressure_relief` (libSystem) returns those pages now. +//! Elsewhere (glibc, Windows) blocks this large are unmapped when freed: nothing to do. +//! +//! This is LightCraft's only crate allowed `unsafe` (one FFI call, in `macos`). Its API is safe, +//! and on failure or on other platforms it simply releases nothing. +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] + +/// Return free allocator pages to the system. Returns the number of bytes released (always 0 on +/// platforms where freed large blocks are already unmapped). Safe to call at any time, from any +/// thread. +pub fn release_free_memory() -> usize { + #[cfg(target_os = "macos")] + { + macos::release() + } + #[cfg(not(target_os = "macos"))] + { + 0 + } +} + +#[cfg(target_os = "macos")] +#[allow(unsafe_code)] +mod macos { + unsafe extern "C" { + /// libSystem malloc: release free memory of `zone` (all zones when null) back to the + /// system, up to `goal` bytes (0 = as much as possible). Returns the bytes released. + fn malloc_zone_pressure_relief(zone: *mut std::ffi::c_void, goal: usize) -> usize; + } + + pub fn release() -> usize { + // SAFETY: `malloc_zone_pressure_relief` is a documented, thread-safe libSystem function + // (malloc/malloc.h). A null zone means "every zone" and a goal of 0 means "as much as + // possible". It only returns already-free pages to the system: it reads and writes no + // memory we own, takes no pointers we must keep alive, and has no failure mode beyond + // releasing 0 bytes. + unsafe { malloc_zone_pressure_relief(std::ptr::null_mut(), 0) } + } +} + +#[cfg(test)] +mod tests { + use super::release_free_memory; + + #[test] + fn releasing_with_nothing_freed_is_harmless() { + // Twice in a row: the second call usually has nothing left to release. + let _ = release_free_memory(); + let _ = release_free_memory(); + } + + #[test] + fn freed_large_blocks_can_be_released_and_memory_stays_usable() { + // Allocate and free well over the allocator's large-block threshold, then release. + let blocks: Vec> = (0..8).map(|i| vec![i as u8; 16 << 20]).collect(); + let sum: u64 = blocks.iter().map(|b| u64::from(b[b.len() - 1])).sum(); + assert_eq!(sum, (0..8u64).sum()); + drop(blocks); + let released = release_free_memory(); + if cfg!(not(target_os = "macos")) { + assert_eq!(released, 0); + } + // The allocator must still work normally afterwards. + let again = vec![7u8; 16 << 20]; + assert_eq!(again[again.len() - 1], 7); + } + + #[test] + fn concurrent_calls_are_safe() { + let handles: Vec<_> = (0..4).map(|_| std::thread::spawn(release_free_memory)).collect(); + for h in handles { + assert!(h.join().is_ok()); + } + } +} diff --git a/xtask/src/layers.rs b/xtask/src/layers.rs index 7ce550f1..b5c63142 100644 --- a/xtask/src/layers.rs +++ b/xtask/src/layers.rs @@ -37,6 +37,7 @@ pub const TABLE: &[(&str, Class)] = &[ ("color", Class::Layer(0)), ("raster", Class::Layer(0)), ("tiff", Class::Layer(0)), + ("sysmem", Class::Layer(0)), ("raw", Class::Layer(1)), ("codecs", Class::Layer(1)), ("meta", Class::Layer(1)),