Bound the wait on the Lua interpreter in tests

BACKLOG.md

@@ -45,9 +45,11 @@ because `LOOP.md` refers to items by number.
 
 8. **`MOD_INT`/`DIV_INT` return the left operand's type** rather than `int`
    (`AttrExprType.java`, the `case MOD_INT` branch), where `caseMathOperation` returns
-   `WurstTypeInt.instance()` for `+`, `-`, `*`. Only observable if something is a proper
-   subtype of int, so it may be harmless — establish whether it is reachable, then either fix
-   it or leave a comment saying why the asymmetry is intended. Small.
+   `WurstTypeInt.instance()` for `+`, `-`, `*`. It *is* reachable: `WurstTypeIntLiteral` is a
+   proper subtype of both int and real, and `caseMathOperation` collapses two literals to `int`
+   precisely so `real r = 1 + 1` stays an error. Returning `leftType` skips that collapse, so
+   `real r = 7 div 2` and `real r = 7 mod 2` should be accepted where `+` is rejected. Confirm
+   with a test first — that is the failing repro — then return `WurstTypeInt.instance()`. Small.
 
 9. **Keep `WURST_LANGUAGE.md` and `CHANGELOG.md` current** as items land. The bounds section
    says nothing about closures, which now work on Jass. Fold this into whichever item changes
@@ -64,6 +66,40 @@ because `LOOP.md` refers to items by number.
     many tests ran before. Not wrong for compiling one map, but it means `.j` cannot be diffed
     across runs to validate a change — only `.lua` can. Fixing it would make Jass diffable.
 
+13. **A bounded generic class cannot be subclassed.** Found while building a repro for item 3;
+    both backends break, differently, on the same program:
+
+        interface Show<T:>
+            function show(T x) returns int
+        implements Show<int>
+            function show(int x) returns int
+                return x
+        class Box<K: Show>
+            K key
+            construct(K k)
+                key = k
+            function size(int extra) returns int
+                return K.show(key) + extra
+            function shift(int extra) returns int
+                return 1000 + extra
+        class SubBox extends Box<int>
+            construct(int k)
+                super(k)
+            override function size(int extra) returns int
+                return super.size(extra) + 100
+        init
+            Box<int> b = new Box<int>(5)
+            Box<int> s = new SubBox(5)
+            if b.size(1) == 6 and b.shift(1) == 1001 and s.size(1) == 106
+                testSuccess()
+
+    Jass fails to compile: `Typevar dispatch not eliminated.` Lua compiles and runs but never
+    reaches `testSuccess`: the override makes `size` dispatched, and the emitted call is
+    `b:Box_size_specialized_integer(1)` while `b` was allocated from the *erased* `Box` table,
+    which binds only `shift`. The specialised table `Box_specialized_integer` has the slot; the
+    instance never gets that table. Split this if the two turn out to have separate causes —
+    the Jass one is loud and probably the smaller of the two.
+
 12. **Standing item, never finished.** When nothing above is left, find the next thing worth
     doing and add it here rather than stopping. Good sources, in order: a test that would have
     caught a bug already found; a place where two mechanisms do the same job and disagree; a
@@ -88,6 +124,12 @@ because `LOOP.md` refers to items by number.
 
 ## Done
 
+- 14. The Lua execution harness no longer hangs. It read the spawned interpreter's stderr to EOF
+  before touching stdout, and never bounded the wait, so a program that filled the stdout pipe
+  deadlocked the whole suite with no output and no timeout — a stray JVM was still sitting on it
+  twenty minutes later. `checkLuaSyntax`, ten lines further down the same file, already drained
+  both pipes concurrently and waited with a timeout; the execution path now uses the same
+  helper. Verified against the item 13 repro: hung indefinitely before, fails in 8s after.
 - 1 + 2. Lua method names are sanitised where they are assigned, not where they are printed.
   `LuaDispatchPreparation.normalizeMethodNames` is the pass that gives one name to a whole
   dispatch group, so it now sanitises before uniquing — two names differing only in characters
@@ -104,6 +146,8 @@ because `LOOP.md` refers to items by number.
 - `%` is real modulo in Wurst; `mod` is integer modulo. `int % 8` types as `real`.
 - Emitted Lua must be byte-identical for identical input (AGENTS.md §8). It is the only
   emitted output that can be diffed across runs — see item 11.
+- A test that hangs looks exactly like a test that is slow. If the suite stops making progress,
+  take a thread dump of the forked worker (`jstack <pid>`) before killing it — it names the line.
 - Method names are not what the frontend called them. `LuaDispatchPreparation` renames a whole
   dispatch group to one name and attaches alias sets, and only then does the backend run. A
   question about which Lua slot something lands in is a question about that pass, not about

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaRunnerTests.java

@@ -0,0 +1,85 @@
+package tests.wurstscript.tests;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+/**
+ * Tests for the harness that runs emitted Lua, rather than for anything it compiles.
+ * <p>
+ * Both cases here used to stop the whole suite instead of failing a test: the runner drained the
+ * spawned interpreter's stderr to the end before reading stdout, and waited for the process without
+ * a bound. A program that fills the stdout pipe blocks writing while the runner blocks reading the
+ * other pipe, and neither side ever moves. There is nothing to see when that happens - no output,
+ * no failing test, no timeout - so it is worth holding onto tests that would notice it coming back.
+ * <p>
+ * Both are Lua only. Run through the Jass configurations as well, the non-terminating one would
+ * hang the interpreter instead, which is the same problem in a place this fix does not reach.
+ */
+public class LuaRunnerTests extends WurstScriptTest {
+
+    /** Long enough that a program which does terminate still does, short enough to wait for. */
+    @Override
+    protected int luaExecutionTimeoutSeconds() {
+        return 5;
+    }
+
+    @Test
+    public void aProgramThatDoesNotTerminateFailsItsOwnTest() {
+        try {
+            test().testLua(true).luaOnly(true).executeProg().lines(
+                "package test",
+                "native testSuccess()",
+                "init",
+                "    var i = 0",
+                "    while true",
+                "        i += 1",
+                "    testSuccess()"
+            );
+            fail("expected the runner to give up on a program that does not terminate");
+        } catch (Error e) {
+            assertTrue(e.getMessage() != null && e.getMessage().contains("did not terminate"),
+                "expected a timeout, got: " + e.getMessage());
+        }
+    }
+
+    /**
+     * Enough lines to fill the pipe buffer several times over. With the streams read one after the
+     * other this deadlocks: the program blocks writing stdout, the runner blocks reading stderr.
+     */
+    @Test
+    public void aProgramThatFloodsStdoutStillFinishes() {
+        test().testLua(true).luaOnly(true).executeProg().lines(
+            "package test",
+            "native testSuccess()",
+            "native println(string s)",
+            "init",
+            "    for i = 1 to 20000",
+            "        println(\"filling the pipe buffer with a reasonably long line of output\")",
+            "    testSuccess()"
+        );
+    }
+
+    /**
+     * The same drain with the output arriving as one line rather than many. This checks that it
+     * finishes, which is what the two cases above check too — it does not observe how much of the
+     * line was retained, and would pass against a collector that kept all of it. Keeping a bounded
+     * amount is worth doing regardless, but a program printing a megabyte without a newline is not
+     * something this suite expects, so it is not worth test-only machinery to assert.
+     */
+    @Test
+    public void aProgramPrintingWithoutNewlinesStillFinishes() {
+        test().testLua(true).luaOnly(true).executeProg().lines(
+            "package test",
+            "native testSuccess()",
+            "native println(string s)",
+            "init",
+            "    var line = \"\"",
+            "    for i = 1 to 4000",
+            "        line += \"this line keeps growing and is never broken by a newline \"",
+            "    println(line)",
+            "    testSuccess()"
+        );
+    }
+}

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/WurstScriptTest.java

@@ -48,6 +48,21 @@
 public class WurstScriptTest {
 
     public static final String TEST_OUTPUT_PATH = "./test-output/";
+
+    /** Generous enough for the slowest test program, short enough that a loop fails the run. */
+    private static final int LUA_EXECUTION_TIMEOUT_SECONDS = 60;
+
+    /**
+     * How much of a program's output is kept for the failure message. Draining never stops - that
+     * is what deadlocks the run - but retaining everything from a program that loops while printing
+     * would exhaust the worker before the timeout fires.
+     */
+    private static final int RETAINED_OUTPUT_LIMIT = 64 * 1024;
+
+    /** Overridden by the tests covering the runner itself, so they need seconds rather than a minute. */
+    protected int luaExecutionTimeoutSeconds() {
+        return LUA_EXECUTION_TIMEOUT_SECONDS;
+    }
     private static volatile String resolvedLuaExecutable;
     private static volatile String resolvedLuacExecutable;
     private static volatile String extractedLuaWin;
@@ -527,7 +542,6 @@ private void translateAndTestLua(String name, boolean executeProg, WurstGui gui,
                     throw new org.testng.SkipException(
                         "Skipped Lua execution (translation and luac syntax check still ran): " + e.getMessage());
                 }
-                String line;
                 // Preload the WC3 Lua runtime (Reforged blizzard.j dump + native shim)
                 // when available, so tests execute against real BJ implementations.
                 // The generated script only installs fallbacks for natives that are
@@ -550,29 +564,27 @@ private void translateAndTestLua(String name, boolean executeProg, WurstGui gui,
                 Process p = Runtime.getRuntime().exec(args);
                 StringBuilder errors = new StringBuilder();
                 StringBuilder output = new StringBuilder();
-                try (BufferedReader input = new BufferedReader(new InputStreamReader(p.getErrorStream()))) {
-                    while ((line = input.readLine()) != null) {
-                        System.err.println(line);
-                        errors.append(line);
-                        errors.append("\n");
-                    }
+                // Both pipes must be drained while the program runs, and the wait must end: a
+                // generated program that loops forever would otherwise hang the whole suite,
+                // and one that fills the stdout pipe would deadlock against a stderr-first read.
+                java.util.concurrent.atomic.AtomicBoolean sawTestSuccess =
+                    new java.util.concurrent.atomic.AtomicBoolean(false);
+                Thread outCollector = collectStreamAsync(p.getInputStream(), output, "testSuccess", sawTestSuccess);
+                Thread errCollector = collectStreamAsync(p.getErrorStream(), errors);
+                if (!p.waitFor(luaExecutionTimeoutSeconds(), TimeUnit.SECONDS)) {
+                    p.destroyForcibly();
+                    throw new Error(currentTestEnv + ": Lua program did not terminate within "
+                        + luaExecutionTimeoutSeconds() + "s: " + luaFile.getName());
                 }
+                outCollector.join();
+                errCollector.join();
 
                 if (errors.length() > 0) {
+                    System.err.print(errors);
                     throw new TestFailException(errors.toString());
                 }
 
-                boolean success = false;
-                try (BufferedReader input = new BufferedReader(new InputStreamReader(p.getInputStream()))) {
-                    while ((line = input.readLine()) != null) {
-                        if (line.equals("testSuccess")) {
-                            success = true;
-                        }
-                        output.append(line);
-                        output.append("\n");
-                    }
-                }
-                if (!success) {
+                if (!sawTestSuccess.get()) {
                     throw new Error(currentTestEnv + ": Succeed function not called");
                 }
             }
@@ -621,11 +633,65 @@ private void checkLuaSyntax(String luacExecutable, File luaFile) throws IOExcept
     }
 
     private Thread collectStreamAsync(InputStream stream, StringBuilder out) {
+        return collectStreamAsync(stream, out, null, null);
+    }
+
+    /**
+     * Drains {@code stream} into {@code out}, keeping only the first {@link #RETAINED_OUTPUT_LIMIT}
+     * characters. Reading never stops — that is what blocks the process and deadlocks the run — but
+     * a program that loops while printing would fill the heap before the timeout could fire.
+     * <p>
+     * Whatever the caller is looking for is recognised here rather than read back out of
+     * {@code out} afterwards, because it may arrive after the limit: a program that prints a
+     * million lines and then succeeds has still succeeded.
+     */
+    private Thread collectStreamAsync(InputStream stream, StringBuilder out,
+                                      String watchedLine, java.util.concurrent.atomic.AtomicBoolean sawWatchedLine) {
         Thread t = new Thread(() -> {
-            try (BufferedReader input = new BufferedReader(new InputStreamReader(stream))) {
-                String line;
-                while ((line = input.readLine()) != null) {
-                    out.append(line).append("\n");
+            // Fixed-size chunks rather than lines: a line is only bounded by what the program
+            // chose to print, and reading one materialises all of it before any limit here could
+            // apply. Nothing below holds more than the retained limit plus one chunk.
+            char[] chunk = new char[8 * 1024];
+            StringBuilder pendingLine = new StringBuilder();
+            boolean lineIsLongerThanWatched = false;
+            boolean truncated = false;
+            try (Reader input = new InputStreamReader(stream)) {
+                int read;
+                while ((read = input.read(chunk)) >= 0) {
+                    if (watchedLine != null) {
+                        for (int i = 0; i < read; i++) {
+                            char c = chunk[i];
+                            if (c == '\n') {
+                                if (!lineIsLongerThanWatched && pendingLine.length() == watchedLine.length()
+                                    && watchedLine.contentEquals(pendingLine)) {
+                                    sawWatchedLine.set(true);
+                                }
+                                pendingLine.setLength(0);
+                                lineIsLongerThanWatched = false;
+                            } else if (c != '\r' && !lineIsLongerThanWatched) {
+                                // Only ever as long as what is being looked for; past that the
+                                // line cannot be it, so there is no reason to keep any of it.
+                                if (pendingLine.length() == watchedLine.length()) {
+                                    lineIsLongerThanWatched = true;
+                                    pendingLine.setLength(0);
+                                } else {
+                                    pendingLine.append(c);
+                                }
+                            }
+                        }
+                    }
+                    int room = RETAINED_OUTPUT_LIMIT - out.length();
+                    if (room > 0) {
+                        out.append(chunk, 0, Math.min(read, room));
+                    }
+                    // Said as soon as anything is dropped, including when that happens part way
+                    // through the last chunk there is: otherwise the output ends at exactly the
+                    // limit and reads as though that were all the program had to say.
+                    if (read > room && !truncated) {
+                        truncated = true;
+                        out.append("\n... further output dropped after ")
+                            .append(RETAINED_OUTPUT_LIMIT).append(" characters\n");
+                    }
                 }
             } catch (IOException ignored) {
             }