Merge pull request #6765 from dotCooCoo/fix/js-finally-sites-gc-test

Scripts: a finally block left early drops its saved throw site

crates/js/src/host_tests.rs

@@ -436,6 +436,104 @@ fn a_marked_host_error_that_passes_through_a_finally_block_says_where_it_was_rai
     assert_eq!(r.error_mark(), Some(7));
 }
 
+/// A realm with a 1 MiB heap, the size that the `finally` blocks of these tests collect garbage in.
+fn small_heap_realm() -> Realm {
+    let mut r = Realm::new();
+    r.set_limits(Limits { fuel: 200_000_000, heap_bytes: 1 << 20, ..Limits::default() });
+    r
+}
+
+/// Begins a run of `source` and advances it one instruction at a time until a `finally` block runs
+/// for an exception whose throw site is saved.
+fn start_until_a_throw_site_is_saved(r: &mut Realm, source: &str) {
+    r.start(source).unwrap();
+    while r.interp().finally_sites.is_empty() {
+        assert_eq!(r.advance(1), Ok(Progress::Paused), "{source}");
+    }
+}
+
+/// The collections that a `finally` block's allocations cause under a small heap limit keep the
+/// site its exception was raised at, and the run's end leaves no saved site behind.
+#[test]
+fn an_error_that_passes_through_a_finally_block_that_collects_garbage_says_where_it_was_raised() {
+    let mut r = small_heap_realm();
+    let source = "try {\n  throw new Error('original');\n} finally {\n  for (var i = 0; i < 20000; i++) { var waste = [i, i + 1, i + 2]; }\n}";
+    start_until_a_throw_site_is_saved(&mut r, source);
+    r.collect_garbage();
+    let collections = r.interp().heap.collections;
+    let e = loop {
+        match r.advance(1_000) {
+            Ok(Progress::Paused) => assert_eq!(r.interp().finally_sites.len(), 1, "the site stays saved while the block runs"),
+            Ok(Progress::Done(v)) => panic!("the run ended with {v:?}"),
+            Err(e) => break e,
+        }
+    };
+    assert!(r.interp().heap.collections > collections, "the block's allocations collect garbage");
+    assert_eq!((e.kind, e.line, e.column, e.message.as_str()), (RunErrorKind::Thrown, 2, 3, "Error: original"));
+    assert!(r.interp().finally_sites.is_empty());
+}
+
+/// The code of a saved throw site stays live through a collection: direct `eval` code that
+/// threw is not part of the code of any frame still running.
+#[test]
+fn a_saved_throw_site_keeps_its_code_live() {
+    let mut r = small_heap_realm();
+    let source = "try {\n  eval(\"var unused = 'padding padding padding padding';\\nthrow new Error('original');\");\n} finally {\n  for (var i = 0; i < 2000; i++) { var waste = [i]; }\n}";
+    start_until_a_throw_site_is_saved(&mut r, source);
+    let with_site = r.collect_garbage();
+    let sites = std::mem::take(&mut r.interp().finally_sites);
+    let without_site = r.collect_garbage();
+    r.interp().finally_sites = sites;
+    assert_eq!(r.collect_garbage(), with_site, "the saved site's code is counted again");
+    assert!(with_site > without_site, "{with_site} <= {without_site}");
+    let e = finish(&mut r, 1_000).0.unwrap_err();
+    assert_eq!((e.kind, e.line, e.message.as_str()), (RunErrorKind::Thrown, 2, "Error: original"));
+    assert!(r.interp().finally_sites.is_empty());
+}
+
+/// A run that a host function stops inside a `finally` block leaves no saved throw site behind.
+#[test]
+fn a_run_stopped_in_a_finally_block_leaves_no_saved_throw_site() {
+    let mut r = realm(1_000_000);
+    let f: HostFunction = Rc::new(|_| Err(HostError::Abort("stopped".into())));
+    r.define_host_object("host", vec![("abort".into(), HostItem::Function(f))]).unwrap();
+    let e = r.run("try { throw new Error('original'); } finally { host.abort(); }").unwrap_err();
+    assert_eq!((e.kind, e.message.as_str()), (RunErrorKind::Limit(LimitKind::Host), "stopped"));
+    assert!(r.interp().finally_sites.is_empty());
+    assert_eq!(r.run("1").unwrap(), JsValue::Number(1.0));
+    assert!(r.interp().finally_sites.is_empty());
+}
+
+/// A `finally` block that `break`, `continue` or `return` leaves drops the saved site of the
+/// exception it ran for as it leaves, before the stack grows back over that site's slot.
+#[test]
+fn leaving_a_finally_block_early_drops_its_saved_throw_site() {
+    let mut r = realm(1_000_000);
+    let marked = Rc::new(Cell::new(false));
+    let seen = marked.clone();
+    let f: HostFunction = Rc::new(move |_| {
+        seen.set(true);
+        Ok(Data::Undefined)
+    });
+    r.define_host_object("host", vec![("mark".into(), HostItem::Function(f))]).unwrap();
+    for source in [
+        "do { try { throw new Error('x'); } finally { break; } } while (false);\nhost.mark(); var a = [1, 2, 3];",
+        "for (var i = 0; i < 1; i++) { try { throw new Error('x'); } finally { continue; } }\nhost.mark(); var a = [1, 2, 3];",
+        "function f() { try { throw new Error('x'); } finally { return 1; } }\nf(); host.mark(); var a = [f, f, f];",
+    ] {
+        marked.set(false);
+        let mut saved = false;
+        r.start(source).unwrap();
+        while r.advance(1).unwrap() == Progress::Paused {
+            saved |= !r.interp().finally_sites.is_empty();
+            if marked.get() {
+                assert!(r.interp().finally_sites.is_empty(), "{source}");
+            }
+        }
+        assert!(saved && marked.get(), "{source}");
+    }
+}
+
 #[test]
 fn advancing_without_a_run_is_an_error() {
     let mut r = realm(1_000);

crates/js/src/interp.rs

@@ -235,9 +235,9 @@ pub(crate) struct Interp {
     /// Where the exceptions `finally` blocks are running for were raised, innermost last, each with
     /// the stack slot that holds the exception while the block runs. The block's end throws the
     /// exception on from that site. An entry whose slot is at or above the stack's length belongs to
-    /// a block that was left, and [`Self::drop_left_finally_sites`] removes it. [`Self::unwind`]
-    /// removes the entries at or above a handler's slot before it adds one, so the slots rise and
-    /// the list is never longer than the stack.
+    /// a block that was left, and [`Self::drop_left_finally_sites`] removes it at each `Jump`, each
+    /// return and each collection. [`Self::unwind`] removes the entries at or above a handler's slot
+    /// before it adds one, so the slots rise and the list is never longer than the stack.
     pub(crate) finally_sites: Vec<(ThrowSite, usize)>,
     /// The mark of the host error the last run ended with.
     pub(crate) error_mark: Option<u32>,
@@ -885,7 +885,11 @@ impl Interp {
             Op::New(n) => self.op_new(n),
             Op::Binary(b) => self.op_binary(b),
             Op::Neg | Op::Plus | Op::BitNot | Op::Not | Op::Typeof | Op::ToNumber => self.op_unary(op),
-            Op::Jump(t) => self.jump_to(t),
+            Op::Jump(t) => {
+                // `break` and `continue` leave a `finally` block by popping its exception and jumping.
+                self.drop_left_finally_sites(self.stack.len());
+                self.jump_to(t)
+            }
             Op::JumpIfFalse(t) | Op::JumpIfTrue(t) => self.op_branch(t, matches!(op, Op::JumpIfTrue(_)), false),
             Op::JumpIfFalseKeep(t) | Op::JumpIfTrueKeep(t) => self.op_branch(t, matches!(op, Op::JumpIfTrueKeep(_)), true),
             Op::Throw => {

docs/target-app-parity.md

@@ -746,6 +746,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   - Scripts: the Script Console panel's report line reads as `script.console` gives it, `time unavailable` included on a platform without a clock; counts of host calls, dropped lines and cut lines take the singular for one, in each language's own plural forms; and a run that is no longer available still gets its end line (`crates/engine/src/tests/script_console.rs`, `crates/ui-egui/src/panels/tests_script_console.rs`).
   - Scripts: an error `$.evalFile` throws for a file it cannot run is reported at that file's line and column only while no `catch` has taken it. Caught and thrown again later, in the script or in another file, it is reported where it was thrown again, and a new error whose message holds its text is reported where that error was thrown. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.rs`.
   - Scripts: an uncaught error that one or more `finally` blocks pass on unchanged, in the script or in a function it called, is reported at the line and column where it was thrown, in `script.run`'s report and the Script Console. An error a `finally` block throws is reported at that throw, and one its `return`, `break` or `continue` discards is not reported. VectorCraft's own rule. `crates/engine/src/tests/script_include.rs`, `crates/js/src/host_tests.rs`.
+  - Scripts: an error a `finally` block passes on is still reported where it was thrown when the block's allocations collect garbage under a small heap limit, and a `finally` block that `return`, `break` or `continue` leaves drops the saved location of its error as it leaves, so a collection no longer counts that code as live. VectorCraft's own rule. `crates/js/src/host_tests.rs`.
   - Scripts: a run reads its preprocessor lines before any of it runs: `#include` and `//@include` put another file in place of the line, looked for beside the including file, in the `#includepath` folders and in `JSINCLUDE`; `#script` names the run; `#target` and `#strict off` are taken, `#strict on` turns on strict write checks (below), `#targetengine` names the engine (below), and `#engine` is refused. `$.evalFile` runs a file with the same globals and returns its last value, and `$.fileName` and `$.includePath` give the file being run and the include folders. A script run from a file reads these files only in its folder and below, without `..` names or links that lead out, and an error in one is reported at its own file, line and column ([preprocessor directives](https://extendscript.docsforadobe.dev/extendscript-tools-features/preprocessor-directives/), [the `$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/); `crates/engine/src/tests/script_include.rs`).
   - Scripts: `#targetengine name` (and `//@targetengine name`) runs the script in a named engine that lasts for the session, and a later script that names it sees the globals and functions earlier ones left ([preprocessor directives](https://extendscript.docsforadobe.dev/extendscript-tools-features/preprocessor-directives/)); `$.engineName` gives the name ([the `$` object](https://extendscript.docsforadobe.dev/extendscript-tools-features/dollar-object/)). VectorCraft's own rules: a script without the line and `#targetengine main` get a new engine each run, the line comes once in the script itself before its code, a session keeps 8 named engines with 64 MiB of heap each, an error or a stop keeps the globals the run set while the documents go back, kept host functions act for the run that calls them, `script.engines` lists the engines and `script.engine.drop` drops one, and the web build has none. `crates/engine/src/tests/script_engines.rs`.
   - Scripts: `File` and `Folder` ([File object](https://extendscript.docsforadobe.dev/file-system-access/file-object/), [Folder object](https://extendscript.docsforadobe.dev/file-system-access/folder-object/)) name files and folders by platform or URI path, with `name`, `fsName`, `absoluteURI`, `relativeURI`, `parent`, `Folder.current` and `encode`/`decode`, and read them: `exists`, `length`, the dates, `getFiles` with a mask or a function, and `open('r')` with `read`, `readln`, `readch`, `seek`, `tell` and `close`, decoding by byte order mark, `encoding`, UTF-8 or CP1252. VectorCraft's own rules: a script reads only in its own folder and below (and inside the automation read roots when they are set); any other path and a network or device path are refused before anything is looked up, and a link that leads out is refused as well (`false` or `null`, with `error` set); a file is read whole up to 8 MiB and a run reads up to 32 MiB; writing, the file dialogs and `execute` are not available yet. `crates/engine/src/tests/script_io.rs`.