M3: the watchdog stops the render it gives up on (#522)

When a render runs past `stuck_after`, the watchdog answers with an error,
starts a replacement worker and drops the late result. The abandoned worker
still rendered the page to the end: #409's stop flag belonged to the whole
pool and was only set when the pool was dropped.

Each worker now has its own stop flag. Dropping the pool sets every worker's
flag, as before. The watchdog sets the flag of the worker it gives up on, so
that render stops at its next content operator. Its answer was already thrown
away, so nothing the user sees changes. Live workers are never stopped.

After giving up on 4 heavy renders, the process used 11.2–11.6 s of CPU in
the next 3 s with nothing left to do; now it uses none.

Co-authored-by: Claude Opus 5.5 <[email protected]>

crates/render/src/raster.rs

@@ -306,9 +306,10 @@ struct Shared {
     /// Pages (and request kinds) the watchdog gave up on: answered with an error at once, so a
     /// pathological page cannot trap every worker in turn.
     stuck: Mutex<std::collections::HashSet<(usize, RequestKind)>>,
-    /// Set when the pool is dropped: the workers' renders stop at their next content operator,
-    /// since nobody can receive their answers any more.
-    dropped: Arc<std::sync::atomic::AtomicBool>,
+    /// Per worker id: set to stop that worker's render at its next content operator once nobody
+    /// can receive its answer (the pool was dropped, or the watchdog gave up on the render).
+    /// Lock `busy` first when holding both.
+    stop: Mutex<Vec<Arc<std::sync::atomic::AtomicBool>>>,
     /// Test hook: make one page slow.
     #[cfg(test)]
     slow_page: Mutex<Option<(usize, std::time::Duration)>>,
@@ -342,9 +343,11 @@ fn take_next(shared: &Shared) -> Option<RenderRequest> {
 ///
 /// **Watchdog:** a render running longer than `stuck_after` (default [`STUCK_AFTER`]) is reported
 /// as an error for that page, the page is not attempted again, and a replacement worker takes
-/// the stuck one's place (threads cannot be killed; the stuck one exits when its render
-/// finally returns, and its late result is dropped). At most `threads` replacements are started
-/// per pool, so a document full of pathological pages cannot spawn threads without bound.
+/// the stuck one's place (threads cannot be killed: the stuck one is told to stop at its next
+/// content operator, exits when its render returns, and its late result is dropped; a single
+/// long operator, such as decoding a huge image, still runs to its end). At most `threads`
+/// replacements are started per pool, so a document full of pathological pages cannot spawn
+/// threads without bound.
 pub struct RenderPool {
     shared: Arc<Shared>,
     wake: Mutex<Vec<Sender<()>>>,
@@ -394,14 +397,16 @@ impl RenderPool {
         return false;
         #[cfg(not(target_arch = "wasm32"))]
         {
-            let id = {
+            let (id, stop) = {
                 let mut busy = lock(&self.shared.busy);
                 busy.push(None);
-                busy.len() - 1
+                let stop = Arc::new(std::sync::atomic::AtomicBool::new(false));
+                lock(&self.shared.stop).push(stop.clone());
+                (busy.len() - 1, stop)
             };
             let (wtx, wrx) = channel::<()>();
             let (shared, out, bytes, config) = (self.shared.clone(), self.results_tx.clone(), self.bytes.clone(), self.config.clone());
-            match std::thread::Builder::new().name(format!("pdfcraft-render-{id}")).spawn(move || worker(id, bytes, config, shared, wrx, out)) {
+            match std::thread::Builder::new().name(format!("pdfcraft-render-{id}")).spawn(move || worker(id, bytes, config, shared, wrx, out, stop)) {
                 Ok(h) => {
                     lock(&self.wake).push(wtx);
                     lock(&self._workers).push(h);
@@ -473,11 +478,15 @@ impl RenderPool {
             let mut gave_up = Vec::new();
             {
                 let mut busy = lock(&self.shared.busy);
-                for slot in busy.iter_mut() {
+                let stop = lock(&self.shared.stop);
+                for (id, slot) in busy.iter_mut().enumerate() {
                     if let Some((req, since)) = *slot
                         && since.elapsed() > self.stuck_after
                     {
                         *slot = None; // the worker sees this and exits when it returns
+                        if let Some(s) = stop.get(id) {
+                            s.store(true, std::sync::atomic::Ordering::Relaxed);
+                        }
                         gave_up.push(req);
                     }
                 }
@@ -510,19 +519,29 @@ impl RenderPool {
 
 impl Drop for RenderPool {
     fn drop(&mut self) {
-        self.shared.dropped.store(true, std::sync::atomic::Ordering::Relaxed);
+        for s in lock(&self.shared.stop).iter() {
+            s.store(true, std::sync::atomic::Ordering::Relaxed);
+        }
     }
 }
 
-/// A worker's interpreter settings: the document's, stopping once the pool is dropped.
+/// A worker's interpreter settings: the document's, stopping once `stop` is set.
 #[cfg(not(target_arch = "wasm32"))]
-fn worker_settings(config: &RenderConfig, shared: &Shared) -> InterpreterSettings {
-    InterpreterSettings { cancelled: Some(shared.dropped.clone()), ..config.settings() }
+fn worker_settings(config: &RenderConfig, stop: &Arc<std::sync::atomic::AtomicBool>) -> InterpreterSettings {
+    InterpreterSettings { cancelled: Some(stop.clone()), ..config.settings() }
 }
 
 #[cfg(not(target_arch = "wasm32"))]
-fn worker(id: usize, bytes: Arc<Vec<u8>>, config: RenderConfig, shared: Arc<Shared>, wake: Receiver<()>, out: Sender<RenderedPage>) {
-    let settings = worker_settings(&config, &shared);
+fn worker(
+    id: usize,
+    bytes: Arc<Vec<u8>>,
+    config: RenderConfig,
+    shared: Arc<Shared>,
+    wake: Receiver<()>,
+    out: Sender<RenderedPage>,
+    stop: Arc<std::sync::atomic::AtomicBool>,
+) {
+    let settings = worker_settings(&config, &stop);
     // Outer loop: (re)build parser + cache; rebuilt after a renderer panic.
     loop {
         let pdf = parse(&bytes, config.password.as_deref());
@@ -1154,9 +1173,9 @@ endstream endobj
 trailer << /Root 1 0 R >>
 %%EOF";
         let pool = RenderPool::new(Arc::new(ONE_PAGE.to_vec()), 1, RenderConfig::default());
-        let settings = worker_settings(&pool.config, &pool.shared);
-        let flag = settings.cancelled.clone().expect("workers can be cancelled");
-        assert!(Arc::ptr_eq(&flag, &pool.shared.dropped));
+        let flag = lock(&pool.shared.stop).first().cloned().expect("one worker");
+        let settings = worker_settings(&pool.config, &flag);
+        assert!(settings.cancelled.as_ref().is_some_and(|c| Arc::ptr_eq(c, &flag)));
         let shapes = Pdf::new(Arc::new(ONE_PAGE.to_vec())).expect("parses");
         let text = Pdf::new(Arc::new(TEXT.to_vec())).expect("parses");
         let pages = shapes.pages();
@@ -1173,6 +1192,26 @@ trailer << /Root 1 0 R >>
         assert!(!blue(&settings) && glyphs(&settings) == 0, "once the pool is gone, interpretation stops before drawing");
     }
 
+    #[test]
+    fn the_watchdog_stops_the_render_it_gives_up_on() {
+        let mut pool = RenderPool::new(Arc::new(ONE_PAGE_TWICE.to_vec()), 1, RenderConfig::default());
+        pool.set_stuck_after(std::time::Duration::from_millis(100));
+        *lock(&pool.shared.slow_page) = Some((0, std::time::Duration::from_millis(1500)));
+        pool.set_queue(vec![RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 0.5, tag: 0 }]);
+        let t = std::time::Instant::now();
+        let first = loop {
+            if let Some(p) = pool.try_recv() {
+                break p;
+            }
+            assert!(t.elapsed() < std::time::Duration::from_secs(8), "no answer");
+            std::thread::sleep(std::time::Duration::from_millis(5));
+        };
+        assert!(first.error.as_deref().is_some_and(|e| e.contains("took longer")), "{:?}", first.error);
+        // The abandoned worker is told to stop at its next operator; its replacement isn't.
+        let stop: Vec<bool> = lock(&pool.shared.stop).iter().map(|s| s.load(std::sync::atomic::Ordering::Relaxed)).collect();
+        assert_eq!(stop, vec![true, false]);
+    }
+
     #[test]
     fn extracts_text_with_positions() {
         let pdf = b"%PDF-1.7

vendor/README.md

@@ -7,7 +7,7 @@ the fix. Vendoring copyleft code is never allowed (plan/adr/0001).
 
 | Crate | Version | Licence | Patches | Test |
 |---|---|---|---|---|
-| hayro-interpret | 0.7.0 | Apache-2.0 OR MIT | (1) select `/AP /N` appearance states via `/AS` (checkboxes, radios, stateful icons) and honour the NoView flag; (2) Type3 Unicode fallback to `/Encoding` glyph names, then printable ASCII; (3) `Type3Glyph::advance_width`; (4) `InterpreterSettings::ocg_overrides` for viewer layer toggles; (5) soft masks whose group lacks `/CS` were dropped (Chrome gradient text); (6) paint nesting cap `MAX_PAINT_NESTING` for tiling patterns and Type 3 glyphs (self-referencing content overflowed the stack); (7) CID width ranges in `/W` and `/W2` clamped to `MAX_CID` (a `0 4294967295 w` range hung); (8) `InterpreterSettings::hide_comments` skips markup annotations but keeps widgets and links (View ▸ Hide all comments); (9) `image_size_ok`: images and masks over 2^28 pixels are not decoded (a fuzzed `/W 4294967295` stencil mask allocated 4 GiB); (10) `MAX_NESTED_PAINTS`: nested paints (forms, Type 3 glyphs and tiling patterns inside one another) share a per-page budget of 50,000 (a glyph showing eight glyphs of its own font, sixteen deep, hung); (11) depends on skrifa 0.47 instead of 0.42 (upstream read-fonts fixed a Type 1 tokenizer that recursed through runs of numbers: a fuzzed font hung or overflowed the stack), with the one API change (`Charset::glyph_id` returns `Option`); (12) annotation appearances whose `/BBox` or `/Matrix` collapses to a line or a point are skipped (mapping them onto `/Rect` divided by zero and handed the device infinite or NaN matrices); (13) a Highlight annotation without a usable normal appearance is drawn from its `/QuadPoints`, `/C` and `/CA` with Multiply blending, as pdfcraft-annot draws highlights (it was not drawn at all); (14) simple-font `/Widths` (`read_widths`): entries only for codes 0–255 (one entry per code below `/FirstChar` was allocated: `4294967295` asked for 34 GB), an entry that isn't a number leaves its code missing instead of ending the array, and a `/LastChar` below `/FirstChar` gives no widths instead of discarding the font for a standard one; (15) the appearance box is mapped onto `/Rect` corner to corner: the translation now accounts for the scale (an appearance whose box didn't start at the origin and differed in size from `/Rect` was drawn shifted); (16) a clip set by `W`/`W*` takes effect after whichever path-painting operator ends the path (`S`, `f`, `B`, … as well as `n`; ISO 32000-2 §8.5.4): it was applied only on `n`, so `re W* S` clipped nothing and the clip stayed pending for a later, unrelated `n`; (17) `InterpreterSettings::cancelled`: once set, interpretation stops before the next content operator (a render pool sets it when dropped, after an edit or on closing, so its renders in progress stop instead of running to the end for answers nobody can receive) | (1) `raster::tests::widget_appearance_states`; (2, 3) `xtask text-oracle` on corpus/pdfjs issue918; (4) `raster::tests::layer_override_hides_content`; (5) `raster::tests::soft_mask_without_group_colour_space_is_applied`; (6) `raster::tests::self_referencing_tiling_pattern_terminates`, `raster::tests::self_referencing_type3_glyph_terminates`; (7) `raster::tests::huge_cid_width_ranges_terminate`; (8) `raster::tests::hiding_comments_keeps_fields`; (9) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (10) `raster::tests::type3_glyphs_that_fan_out_into_themselves_finish`; (11) `raster::tests::type1_font_programs_with_long_runs_of_numbers_load_quickly`; (12) `raster::tests::degenerate_annotation_appearances_are_skipped`; (13) `raster::tests::highlights_without_an_appearance_are_drawn`; (14) `raster::tests::malformed_simple_font_widths_keep_later_widths_and_the_font`, `raster::tests::huge_simple_font_first_char_terminates`; (15) `raster::tests::annotation_appearance_box_maps_onto_rect`; (16) `raster::tests::clipping_path_applies_after_any_painting_operator`; (17) `raster::tests::dropping_the_pool_cancels_its_renders` |
+| hayro-interpret | 0.7.0 | Apache-2.0 OR MIT | (1) select `/AP /N` appearance states via `/AS` (checkboxes, radios, stateful icons) and honour the NoView flag; (2) Type3 Unicode fallback to `/Encoding` glyph names, then printable ASCII; (3) `Type3Glyph::advance_width`; (4) `InterpreterSettings::ocg_overrides` for viewer layer toggles; (5) soft masks whose group lacks `/CS` were dropped (Chrome gradient text); (6) paint nesting cap `MAX_PAINT_NESTING` for tiling patterns and Type 3 glyphs (self-referencing content overflowed the stack); (7) CID width ranges in `/W` and `/W2` clamped to `MAX_CID` (a `0 4294967295 w` range hung); (8) `InterpreterSettings::hide_comments` skips markup annotations but keeps widgets and links (View ▸ Hide all comments); (9) `image_size_ok`: images and masks over 2^28 pixels are not decoded (a fuzzed `/W 4294967295` stencil mask allocated 4 GiB); (10) `MAX_NESTED_PAINTS`: nested paints (forms, Type 3 glyphs and tiling patterns inside one another) share a per-page budget of 50,000 (a glyph showing eight glyphs of its own font, sixteen deep, hung); (11) depends on skrifa 0.47 instead of 0.42 (upstream read-fonts fixed a Type 1 tokenizer that recursed through runs of numbers: a fuzzed font hung or overflowed the stack), with the one API change (`Charset::glyph_id` returns `Option`); (12) annotation appearances whose `/BBox` or `/Matrix` collapses to a line or a point are skipped (mapping them onto `/Rect` divided by zero and handed the device infinite or NaN matrices); (13) a Highlight annotation without a usable normal appearance is drawn from its `/QuadPoints`, `/C` and `/CA` with Multiply blending, as pdfcraft-annot draws highlights (it was not drawn at all); (14) simple-font `/Widths` (`read_widths`): entries only for codes 0–255 (one entry per code below `/FirstChar` was allocated: `4294967295` asked for 34 GB), an entry that isn't a number leaves its code missing instead of ending the array, and a `/LastChar` below `/FirstChar` gives no widths instead of discarding the font for a standard one; (15) the appearance box is mapped onto `/Rect` corner to corner: the translation now accounts for the scale (an appearance whose box didn't start at the origin and differed in size from `/Rect` was drawn shifted); (16) a clip set by `W`/`W*` takes effect after whichever path-painting operator ends the path (`S`, `f`, `B`, … as well as `n`; ISO 32000-2 §8.5.4): it was applied only on `n`, so `re W* S` clipped nothing and the clip stayed pending for a later, unrelated `n`; (17) `InterpreterSettings::cancelled`: once set, interpretation stops before the next content operator (a render pool sets it for every worker when dropped, after an edit or on closing, and for one worker when the watchdog gives up on its render, so renders whose answers nobody can receive stop instead of running to the end) | (1) `raster::tests::widget_appearance_states`; (2, 3) `xtask text-oracle` on corpus/pdfjs issue918; (4) `raster::tests::layer_override_hides_content`; (5) `raster::tests::soft_mask_without_group_colour_space_is_applied`; (6) `raster::tests::self_referencing_tiling_pattern_terminates`, `raster::tests::self_referencing_type3_glyph_terminates`; (7) `raster::tests::huge_cid_width_ranges_terminate`; (8) `raster::tests::hiding_comments_keeps_fields`; (9) `raster::tests::absurd_mask_and_fax_sizes_are_refused_before_decoding`; (10) `raster::tests::type3_glyphs_that_fan_out_into_themselves_finish`; (11) `raster::tests::type1_font_programs_with_long_runs_of_numbers_load_quickly`; (12) `raster::tests::degenerate_annotation_appearances_are_skipped`; (13) `raster::tests::highlights_without_an_appearance_are_drawn`; (14) `raster::tests::malformed_simple_font_widths_keep_later_widths_and_the_font`, `raster::tests::huge_simple_font_first_char_terminates`; (15) `raster::tests::annotation_appearance_box_maps_onto_rect`; (16) `raster::tests::clipping_path_applies_after_any_painting_operator`; (17) `raster::tests::dropping_the_pool_cancels_its_renders`, `raster::tests::the_watchdog_stops_the_render_it_gives_up_on` |
 
 | hayro | 0.7.1 | Apache-2.0 OR MIT | (1) `RenderSettings::{x_offset, y_offset}` to render one tile of a large page; (2) images over `MAX_IMAGE_PIXELS` (2^28) are skipped (a fuzzed `/W 4294967295` hung in resampling); (3) `tiling_cell_scale` keeps a tiling pattern's cell pixmap within 3000 px a side whatever its `/XStep`/`/YStep` (a fuzzed step far beyond `/BBox` allocated 2 GB); (4) stroke widths clamped to 4 × (canvas width + height) device pixels (a fuzzed `/LW 9223372036854775807` allocated 10 GB in stroke expansion); (5) `usable_dash_pattern`: a stroke whose dash pattern could cut it into more than `MAX_DASHES_PER_STROKE` (250,000) pieces, counting kurbo's restart of the pattern at every subpath and `ClosePath`, is drawn solid (a fuzzed `/D [[11] 0]` on a line from x = 92234775807 allocated over 5 GB, `[0.000001] 0 d` on a 20 pt line over 10 GB); (6) a dash pattern with a negative or non-finite entry, or a period that isn't positive, is drawn solid (`[-1 -1] 0 d` looped for ever in kurbo) | (1) `raster::tests::tiles_match_full_render`; (2) `raster::tests::absurd_image_dimensions_are_skipped`; (3) `raster::tests::tiling_cells_stay_small_whatever_the_step`; (4) `raster::tests::absurd_predictor_columns_and_line_widths_render`; (5) `raster::tests::dash_patterns_that_cut_a_stroke_into_billions_of_pieces_render`; (6) `raster::tests::invalid_dash_patterns_terminate_and_are_drawn_solid` |
 | hayro-jbig2 | 0.3.0 | Apache-2.0 OR MIT | (1) at most 2^28 pixels per bitmap, page bitmap, halftone grid and grayscale image (each side could be 65535: 4.3 billion pixels decoded one by one); (2) at most 2^26 symbol pixels per symbol dictionary (a fuzzed dictionary decoded for minutes); (3) a symbol dictionary's height classes are capped at one per new symbol and its export runs at `2 × symbols + 1`, each plus `EXTRA_EMPTY_STEPS` (1024): an empty height class or a zero-length export run doesn't advance, and past the end of its data the arithmetic decoder can decode them for ever (one changed byte hung the renderer) | (1) `raster::tests::jbig2_regions_of_billions_of_pixels_are_refused`; (2) the nightly `cargo xtask fuzz` finding, which now finishes in about a second; (3) `raster::tests::jbig2_symbol_dictionaries_that_never_advance_terminate` |