mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-02 06:27:53 -06:00
Never crash: the work gate's byte count saturates instead of overflowing (#2576)
WorkGate::acquire and acquire_urgent added `bytes` with plain `+`. Callers weigh work with saturating maths (export weights, raw denoise), so `bytes` can be usize::MAX: a debug build panicked with the gate's mutex held, and a release build wrapped the count, let the work in and left the accounting wrong until it ended. Both now use saturating_add (a Permit's drop already used saturating_sub), so a holder of usize::MAX makes later work wait. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
23e181283c
commit
f2d36c59e6
@@ -2175,6 +2175,29 @@ mod tests {
|
||||
drop(g.acquire(1000));
|
||||
}
|
||||
|
||||
/// Issue #2576: work weighed with saturating maths can ask for `usize::MAX` bytes. Adding to
|
||||
/// that overflowed (a panic with the gate's mutex held in a debug build, a wrapped count that
|
||||
/// let everything in for a release one); the count now saturates, and later work waits.
|
||||
#[test]
|
||||
fn work_gate_saturates_instead_of_overflowing() {
|
||||
use crate::memory::WorkGate;
|
||||
let g = Arc::new(WorkGate::new(100));
|
||||
let huge = g.acquire(usize::MAX);
|
||||
let urgent = g.acquire_urgent(1);
|
||||
assert_eq!(g.usage().0, usize::MAX);
|
||||
drop(urgent);
|
||||
let g2 = g.clone();
|
||||
let t = std::thread::spawn(move || {
|
||||
let _b = g2.acquire(1);
|
||||
g2.usage().0
|
||||
});
|
||||
std::thread::sleep(std::time::Duration::from_millis(50));
|
||||
assert!(!t.is_finished(), "the second holder waits while the first holds everything");
|
||||
drop(huge);
|
||||
assert_eq!(t.join().unwrap(), 1);
|
||||
assert_eq!(g.usage().0, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn levels_cover_sizes_and_large_exports_use_the_original() {
|
||||
assert_eq!(SourceLevel::for_size(256), SourceLevel::Thumb);
|
||||
|
||||
@@ -128,19 +128,22 @@ impl WorkGate {
|
||||
*self.lock()
|
||||
}
|
||||
|
||||
/// Hold `bytes`, waiting until they fit (or nothing else is held).
|
||||
/// Hold `bytes`, waiting until they fit (or nothing else is held). Callers weigh work with
|
||||
/// saturating maths, so `bytes` may be `usize::MAX`: the count saturates there, never overflows
|
||||
/// (issue #2576).
|
||||
pub fn acquire(&self, bytes: usize) -> Permit<'_> {
|
||||
let mut g = self.lock();
|
||||
while g.0 > 0 && g.0 + bytes > g.1 {
|
||||
while g.0 > 0 && g.0.saturating_add(bytes) > g.1 {
|
||||
g = self.cv.wait(g).unwrap_or_else(|e| e.into_inner());
|
||||
}
|
||||
g.0 += bytes;
|
||||
g.0 = g.0.saturating_add(bytes);
|
||||
Permit { gate: self, bytes }
|
||||
}
|
||||
|
||||
/// Hold `bytes` without waiting (interactive work): counted, so background work waits for it.
|
||||
pub fn acquire_urgent(&self, bytes: usize) -> Permit<'_> {
|
||||
self.lock().0 += bytes;
|
||||
let mut g = self.lock();
|
||||
g.0 = g.0.saturating_add(bytes);
|
||||
Permit { gate: self, bytes }
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user