Fix more stack exhaustion scenarios

Invoking external programs with DeskTop and Selector both
"leaked" stack space. Plug those holes by resetting the stack
before launching anything external.

Author: inexorabletash

RELEASE-NOTES.md

@@ -33,9 +33,11 @@ Project Page: https://github.com/a2stuff/a2d
 * Add Apple+Escape as shortcut to clear selection.
 * Prevent volume icon flicker on startup with empty Disk II drives.
 * Don't show About dialog when Ctrl+Shift+2 is pressed.
+* Fix hang after 60 restarts.
 
 ### Selector
 
+* Fix potential hang after 30 restarts.
 
 ### Disk Copy
 

src/desktop/main.s

@@ -4930,6 +4930,10 @@ show_unexpected_errors_flag:
         ;; Restore system state: devices, /RAM, ROM/ZP banks.
         jsr     RestoreSystem
 
+        ;; Reset stack
+        ldx     #$FF
+        txs
+
         ;; also used by launcher code
         target := *+1
         jmp     SELF_MODIFIED

src/inc/prodos.inc

@@ -488,15 +488,16 @@ int_num:        .byte   0
 .endparams
 .endmacro
 
-.macro DEFINE_QUIT_PARAMS name, ext, pathname
+.macro DEFINE_QUIT_PARAMS name, type, pathname
         WARN_IF_SHADOWING name
     .ifnblank pathname
         .assert pathname < $C000, error, "MLI params can't be in LC RAM"
     .endif
 .params name
 param_count:    .byte   4
-    .ifnblank ext
-        .byte   ext
+quit_type:
+    .ifnblank type
+        .byte   type
     .else
         .byte   0
     .endif

src/lib/invoker.s

@@ -22,11 +22,11 @@
 
         ;; Set prefix
         MLI_CALL SET_PREFIX, invoker::set_prefix_params
-        bcs     ret
+        bcs     quit
 
         ;; Check file type
         MLI_CALL GET_FILE_INFO, invoker::get_info_params
-        bcs     ret
+        bcs     quit
         lda     invoker::get_info_params::file_type
 
 ;;; Binary file (BIN) - load and invoke at A$=AuxType
@@ -55,10 +55,12 @@
 
 ;;; ProDOS 16 System file (S16) - invoke via QUIT call
         cmp     #FT_S16
-        beq     quit_call
+        bne     quit
+        copy8   #$EE, invoker::quit_params::quit_type
+        FALL_THROUGH_TO quit
 
-        ;; Failure
-ret:    rts
+quit:
+        MLI_CALL QUIT, invoker::quit_params
 
 ;;; ============================================================
 ;;; Interpreter - `INVOKER_INTERPRETER` populated by caller
@@ -86,19 +88,19 @@ use_interpreter:
 
 load_target:
         MLI_CALL OPEN, invoker::open_params
-        bcs     exit
+        bcs     quit
         lda     invoker::open_params::ref_num
         sta     invoker::read_params::ref_num
         MLI_CALL READ, invoker::read_params
-        bcs     exit
+        bcs     quit
         MLI_CALL CLOSE, invoker::close_params
-        bcs     exit
+        bcs     quit
 
         ;; If interpreter, copy filename to interpreter buffer.
         lda     INVOKER_INTERPRETER
     IF NOT_ZERO
         MLI_CALL SET_PREFIX, invoker::set_prefix_params
-        bcs     exit
+        bcs     quit
         ldy     INVOKER_FILENAME
       DO
         lda     INVOKER_FILENAME,y
@@ -136,16 +138,11 @@ load_target:
 
         ;; Set return address to the QUIT call below
         ;; (mostly for invoking BIN files)
-        PUSH_RETURN_ADDRESS quit_call
+        PUSH_RETURN_ADDRESS quit
 
         jmp_addr := *+1
         jmp     PRODOS_SYS_START
 
-quit_call:
-        MLI_CALL QUIT, invoker::quit_params
-
-exit:   rts
-
 ;;; ============================================================
 
         PAD_TO ::INVOKER_INTERPRETER
@@ -166,7 +163,7 @@ exit:   rts
         DEFINE_GET_FILE_INFO_PARAMS get_info_params, INVOKER_FILENAME
 
         ;; $EE = extended call signature for IIgs/GS/OS variation.
-        DEFINE_QUIT_PARAMS quit_params, $EE, INVOKER_FILENAME
+        DEFINE_QUIT_PARAMS quit_params, $0, INVOKER_FILENAME
 
 
 .endscope ; invoker

src/selector/app.s

@@ -1591,14 +1591,11 @@ check_path:
 
         jsr     RestoreSystem
 
-        jsr     INVOKER
-
-        ;; If we got here, invoker failed somehow. Relaunch.
-        jsr     Bell
-        jsr     Bell
-        jsr     Bell
-        MLI_CALL QUIT, quit_params
-        brk
+        ;; Reset stack
+        ldx     #$FF
+        txs
+
+        jmp     INVOKER
 
 .endproc ; LaunchPath
 

tests/lib/test.lua

@@ -128,6 +128,14 @@ function test.ExpectLessThanOrEqual(a, b, message, options, level)
   test.Expect(a <= b, message .. " - actual " .. format(a) .. " should be <= " .. format(b), options, inc(level))
 end
 
+function test.ExpectGreaterThan(a, b, message, options, level)
+  test.Expect(a > b, message .. " - actual " .. format(a) .. " should be > " .. format(b), options, inc(level))
+end
+
+function test.ExpectGreaterThanOrEqual(a, b, message, options, level)
+  test.Expect(a >= b, message .. " - actual " .. format(a) .. " should be >= " .. format(b), options, inc(level))
+end
+
 function test.ExpectError(pattern, func, message, options, level)
   local status, err = pcall(func)
   test.Expect(not status, "saw no error; " .. message, options, inc(level))

tests/stack_exhaustion.lua

@@ -11,16 +11,61 @@ DISKARGS="-hard1 $HARDIMG"
 -- however.
 
 test.Step(
-  "Stack exhaustion (takes about 122 seconds to run)",
+  "Restarting launcher (takes about 122 seconds to run)",
   function()
-    local cpu = manager.machine.devices[":maincpu"]
+    a2d.OpenPath("/A2.DESKTOP")
 
     -- Prior to fix, crashes around iteration 40
-    for i = 1,50 do
-      --print(string.format("i=%d SP=%04X", i, cpu.state.SP.value))
-      a2d.CloseAllWindows()
-      a2d.OpenPath("/A2.DESKTOP/DESKTOP.SYSTEM")
+    local cpu = manager.machine.devices[":maincpu"]
+    for i = 1, 50 do
+      a2d.SelectAndOpen("DESKTOP.SYSTEM")
+      a2d.WaitForRestart()
+      print(string.format("i=%d SP=%02X", i, cpu.state.SP.value))
+      test.Expect(not apple2.IsCrashedToMonitor(), "should not have crashed to monitor")
+      test.ExpectGreaterThan(cpu.state.SP.value, 0x120, "stack should not be exausted")
+    end
+
+    a2d.CloseAllWindows()
+end)
+
+test.Step(
+  "Running external programs - DeskTop",
+  function()
+    a2d.OpenPath("/A2.DESKTOP/SAMPLE.MEDIA")
+
+    -- Prior to fix, hangs around iteration 60 (but doesn't crash to monitor)
+    local cpu = manager.machine.devices[":maincpu"]
+    for i = 1, 70 do
+      a2d.SelectAndOpen("KARATEKA.YELL")
       a2d.WaitForRestart()
+      print(string.format("i=%d SP=%02X", i, cpu.state.SP.value))
       test.Expect(not apple2.IsCrashedToMonitor(), "should not have crashed to monitor")
+      test.ExpectGreaterThan(cpu.state.SP.value, 0x120, "stack should not be exausted")
     end
+
+    a2d.CloseAllWindows()
+end)
+
+test.Step(
+  "Running external programs - Selector",
+  function()
+    a2d.AddShortcut("/A2.DESKTOP/SAMPLE.MEDIA/KARATEKA.YELL")
+    a2d.ToggleOptionShowShortcutsOnStartup()
+    a2d.Reboot()
+
+    -- Prior to fix, runs out of stack around iteration 30 (but doesn't crash!)
+    local cpu = manager.machine.devices[":maincpu"]
+    for i = 1, 40 do
+      apple2.Type("1")
+      a2d.DialogOK()
+      a2d.WaitForRestart()
+      print(string.format("i=%d SP=%02X", i, cpu.state.SP.value))
+      test.Expect(not apple2.IsCrashedToMonitor(), "should not have crashed to monitor")
+      test.ExpectGreaterThan(cpu.state.SP.value, 0x120, "stack should not be exausted")
+    end
+
+    apple2.Type("D") -- Desktop
+    a2d.WaitForRestart()
+    a2d.DeletePath("/A2.DESKTOP/LOCAL")
+    a2d.Reboot()
 end)