FormCalc: a script its own clock stops is abandoned, as when the wait runs out first (#6783)

A FormCalc script under a time limit is watched twice: the caller waits
`timeout` for it, and the interpreter reads its own deadline, set to the
same `timeout`, every 256 steps. Which one notices first is down to the
scheduler, and the two reported different things: the caller's outcome is
abandoned with no changes (the form's scripts go off), while the
interpreter's kept every change the script made before the deadline.

For a click script that loops on addInstance, that difference is 1000 rows
(the most one run may add). Since repeating subforms lay out up to 1000
rows instead of 50 (#4608, landed with #6716), keeping them means laying
out a 1000-row form again, twice, which takes most of a minute in the test
build. The engine test hostile_formcalc_scripts_cannot_hang_or_bloat_the_document
took 3.0 s when the caller noticed first and 46-61 s when the script did,
so it failed its 60 s bound at random.

Now a run stopped by its own clock reports what an abandoned run does: the
same error, no changes, abandoned. XfaSession treats that outcome like a
wait that ran out (the session is dead after it). Limits other than time
(steps, loops, depth) keep their changes as before.

Co-authored-by: FredApps <FredApps@users.noreply.github.com>

crates/js/src/formcalc.rs

@@ -100,6 +100,9 @@ struct Interp<'h> {
     /// burning a core to the loop limit).
     #[cfg(not(target_arch = "wasm32"))]
     deadline: Option<std::time::Instant>,
+    /// `deadline` passed and stopped the script: the run reports as abandoned.
+    #[cfg(not(target_arch = "wasm32"))]
+    out_of_time: bool,
 }
 
 type R<T> = Result<T, String>;
@@ -151,6 +154,8 @@ pub(crate) fn run_formcalc_at(
         exited: false,
         #[cfg(not(target_arch = "wasm32"))]
         deadline: deadline.and_then(|d| std::time::Instant::now().checked_add(d)),
+        #[cfg(not(target_arch = "wasm32"))]
+        out_of_time: false,
     };
     let mut out = XfaOutcome::default();
     match interp.block(&body) {
@@ -165,6 +170,19 @@ pub(crate) fn run_formcalc_at(
         }
         Err(e) => out.error = Some(e),
     }
+    // The caller stops waiting at the same time limit, so which of the two notices first is
+    // down to the scheduler. The outcome must not depend on it: stopped by the clock, the run
+    // reports what an abandoned one does, the error and no changes. Keeping the changes made
+    // it a race: a loop adding rows until the clock stopped it left the form unchanged on one
+    // run and 1000 rows longer on the next (each laid out again, tens of seconds).
+    #[cfg(not(target_arch = "wasm32"))]
+    if let (true, Some(d)) = (interp.out_of_time, deadline) {
+        return XfaOutcome {
+            error: Some(format!("the script ran longer than {:.1} s and was abandoned", d.as_secs_f64())),
+            abandoned: true,
+            ..Default::default()
+        };
+    }
     out.notes = host.notes();
     out.effects = host.effects;
     out.console = host.console;

crates/js/src/formcalc/access.rs

@@ -25,6 +25,7 @@ impl Interp<'_> {
             && let Some(d) = self.deadline
             && std::time::Instant::now() > d
         {
+            self.out_of_time = true;
             return Err(RAN_TOO_LONG.into());
         }
         #[cfg(target_arch = "wasm32")]

crates/js/src/tests.rs

@@ -1637,7 +1637,21 @@ $host.gotoURL("https://example.org")"#,
             Limits { loop_iterations: u64::MAX, recursion: 32 },
             Some(std::time::Duration::ZERO),
         );
-        assert!(o.error.unwrap().contains("too long"));
+        assert!(o.abandoned && o.error.as_deref().is_some_and(|e| e.contains("ran longer")), "{o:?}");
+        // Stopped by its clock, a script keeps none of its changes, as when the caller stops
+        // waiting first: the outcome is the same whichever side notices. A loop adding rows
+        // until the clock stops it kept up to 1000 new rows when the script noticed first.
+        let o = crate::formcalc::run_formcalc_at(
+            "while (1) do table._row.addInstance(1) endwhile",
+            &ev,
+            &doc,
+            std::sync::Arc::new(form()),
+            false,
+            Limits { loop_iterations: u64::MAX, recursion: 32 },
+            Some(std::time::Duration::from_millis(50)),
+        );
+        assert!(o.abandoned && o.effects.is_empty() && o.notes.is_empty(), "{:?}, {} effects", o.error, o.effects.len());
+        assert!(o.error.as_deref().is_some_and(|e| e.contains("ran longer")), "{:?}", o.error);
         assert!(run_hostile_formcalc(&format!("{}1{}", "-".repeat(5000), "")).error.is_some());
         assert!(run_hostile_formcalc("var s = \"x\"\nfor i = 1 upto 100 do s = Concat(s, s) endfor\nLen(s)").error.unwrap().contains("too long"));
         assert!(run_hostile_formcalc("Space(1e12)").error.is_none());
@@ -1885,6 +1899,16 @@ mod session {
         slow.timeout = std::time::Duration::from_millis(200);
         let o = s.run(slow);
         assert!(o.abandoned || o.error.as_deref().is_some_and(|e| e.contains("too long")), "{o:?}");
+        // Whether the script's clock or the session's wait notices first, a FormCalc loop
+        // adding rows until the time is up is abandoned with none of its rows, and the session
+        // is dead after it.
+        let mut s = XfaSession::new(XfaDoc::default());
+        let mut slow = script("while (1) do table._row.addInstance(1) endwhile", true, "form1[0].page1[0].go[0]", "click");
+        slow.limits = Limits { loop_iterations: u64::MAX, recursion: 32 };
+        slow.timeout = std::time::Duration::from_millis(200);
+        let o = s.run(slow);
+        assert!(o.abandoned && o.effects.is_empty(), "{:?}, {} effects", o.error, o.effects.len());
+        assert!(s.dead().is_some_and(|w| w.contains("abandoned")));
     }
 }
 

crates/js/src/xfa_session.rs

@@ -179,7 +179,15 @@ mod native {
             loop {
                 let left = deadline.map_or(limit, |d| d.saturating_duration_since(Instant::now()));
                 match worker.from.recv_timeout(left) {
-                    Ok(Reply::Outcome(o)) => return *o,
+                    Ok(Reply::Outcome(o)) => {
+                        // A FormCalc script its own clock stopped reports as abandoned: the
+                        // session ends as it does when the wait runs out first.
+                        if o.abandoned {
+                            self.dead = o.error.clone();
+                            self.worker = None;
+                        }
+                        return *o;
+                    }
                     Ok(Reply::Started) => {
                         // A second call changes nothing: the script's time has already begun.
                         if !running {