Don't clear interrupt until the stack is empty (#405)

* Don't clear interrupt until the stack is empty

Previous to this the interrupt would be cleared whenever it is noticed
before it starts returning the error up the stack.

Unfortunately if you have goja code that called go code that called a
goja code and it got interrupted. Once we get back in the go code we
just got an error, and it's very likely that the code will just return
it
so it's just an exception.

So at that point if there is `try/catch` it will just catch this and the
code will not actually be interrupted.

It is possible to at each such occurrence test that the error is not
InterruptError and Interrupt again, but that seems like the worse of two
choices.

So instead now the Interrupt will *not* be cleared and if the go code
just propagates the error - it will just keep interrupting.

But it is still possible to check the error in the go code and decide to
clear the interrupt manually.

The interrupt will be cleared only once the stack is empty,
so there is no way for the above problem.

It will also clear the job queue. Otherwise, if there is any promise
that has been resolved/rejected, and they aren't cleared they will be
executed the next time the Runtime is used, and the stack is empty.

* Update runtime.go

Co-authored-by: Dmitry Panov <[email protected]>

Co-authored-by: Dmitry Panov <[email protected]>

runtime.go

@@ -1367,6 +1367,9 @@ func (r *Runtime) RunProgram(p *Program) (result Value, err error) {
 		if x := recover(); x != nil {
 			if ex, ok := x.(*uncatchableException); ok {
 				err = ex.err
+				if len(r.vm.callStack) == 0 {
+					r.leaveAbrupt()
+				}
 			} else {
 				panic(x)
 			}
@@ -1423,6 +1426,8 @@ func (r *Runtime) CaptureCallStack(depth int, stack []StackFrame) []StackFrame {
 }
 
 // Interrupt a running JavaScript. The corresponding Go call will return an *InterruptedError containing v.
+// If the interrupt propagates until the stack is empty the currently queued promise resolve/reject jobs will be cleared
+// without being executed. This is the same time they would be executed otherwise.
 // Note, it only works while in JavaScript code, it does not interrupt native Go functions (which includes all built-ins).
 // If the runtime is currently not running, it will be immediately interrupted on the next Run*() call.
 // To avoid that use ClearInterrupt()
@@ -2329,6 +2334,9 @@ func AssertFunction(v Value) (Callable, bool) {
 					if x := recover(); x != nil {
 						if ex, ok := x.(*uncatchableException); ok {
 							err = ex.err
+							if len(obj.runtime.vm.callStack) == 0 {
+								obj.runtime.leaveAbrupt()
+							}
 						} else {
 							panic(x)
 						}
@@ -2613,7 +2621,7 @@ func (r *Runtime) getHash() *maphash.Hash {
 	return r.hash
 }
 
-// called when the top level function returns (i.e. control is passed outside the Runtime).
+// called when the top level function returns normally (i.e. control is passed outside the Runtime).
 func (r *Runtime) leave() {
 	for {
 		jobs := r.jobQueue
@@ -2627,6 +2635,12 @@ func (r *Runtime) leave() {
 	}
 }
 
+// called when the top level function returns (i.e. control is passed outside the Runtime) but it was due to an interrupt
+func (r *Runtime) leaveAbrupt() {
+	r.jobQueue = nil
+	r.ClearInterrupt()
+}
+
 func nilSafe(v Value) Value {
 	if v != nil {
 		return v

runtime_test.go

@@ -1500,6 +1500,121 @@ func TestInterruptInWrappedFunction(t *testing.T) {
 	}
 }
 
+func TestInterruptInWrappedFunction2(t *testing.T) {
+	rt := New()
+	// this test panics as otherwise goja will recover and possibly loop
+	var called bool
+	rt.Set("v", rt.ToValue(func() {
+		if called {
+			go func() {
+				panic("this should never get called twice")
+			}()
+		}
+		called = true
+		rt.Interrupt("here is the error")
+	}))
+
+	rt.Set("s", rt.ToValue(func(a Callable) (Value, error) {
+		return a(nil)
+	}))
+
+	rt.Set("k", rt.ToValue(func(e Value) {
+		go func() {
+			panic("this should never get called actually")
+		}()
+	}))
+	_, err := rt.RunString(`
+        Promise.resolve().then(()=>k()); // this should never resolve
+        while(true) {
+            try{
+                s(() =>{
+                    v();
+                })
+                break;
+            } catch (e) {
+                k(e);
+            }
+        }
+	`)
+	if err == nil {
+		t.Fatal("expected error but got no error")
+	}
+	intErr := new(InterruptedError)
+	if !errors.As(err, &intErr) {
+		t.Fatalf("Wrong error type: %T", err)
+	}
+	if !strings.Contains(intErr.Error(), "here is the error") {
+		t.Fatalf("Wrong error message: %q", intErr.Error())
+	}
+	_, err = rt.RunString(`Promise.resolve().then(()=>globalThis.S=5)`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := rt.Get("S")
+	if s == nil || s.ToInteger() != 5 {
+		t.Fatalf("Wrong value for S %v", s)
+	}
+}
+
+func TestInterruptInWrappedFunction2Recover(t *testing.T) {
+	rt := New()
+	// this test panics as otherwise goja will recover and possibly loop
+	var vCalled int
+	rt.Set("v", rt.ToValue(func() {
+		if vCalled == 0 {
+			rt.Interrupt("here is the error")
+		}
+		vCalled++
+	}))
+
+	rt.Set("s", rt.ToValue(func(a Callable) (Value, error) {
+		v, err := a(nil)
+		if err != nil {
+			intErr := new(InterruptedError)
+			if errors.As(err, &intErr) {
+				rt.ClearInterrupt()
+				return nil, errors.New("oops we got interrupted let's not that")
+			}
+		}
+		return v, err
+	}))
+	var kCalled int
+
+	rt.Set("k", rt.ToValue(func(e Value) {
+		kCalled++
+	}))
+	_, err := rt.RunString(`
+        Promise.resolve().then(()=>k());
+        while(true) {
+            try{
+                s(() => {
+                    v();
+                })
+                break;
+            } catch (e) {
+                k(e);
+            }
+        }
+	`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if vCalled != 2 {
+		t.Fatalf("v was not called exactly twice but %d times", vCalled)
+	}
+	if kCalled != 2 {
+		t.Fatalf("k was not called exactly twice but %d times", kCalled)
+	}
+	_, err = rt.RunString(`Promise.resolve().then(()=>globalThis.S=5)`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := rt.Get("S")
+	if s == nil || s.ToInteger() != 5 {
+		t.Fatalf("Wrong value for S %v", s)
+	}
+}
+
 func TestRunLoopPreempt(t *testing.T) {
 	vm := New()
 	v, err := vm.RunString("(function() {for (;;) {}})")

vm.go

@@ -419,8 +419,6 @@ func (vm *vm) run() {
 			iface: vm.interruptVal,
 		}
 		v.stack = vm.captureStack(nil, 0)
-		atomic.StoreUint32(&vm.interrupted, 0)
-		vm.interruptVal = nil
 		vm.interruptLock.Unlock()
 		panic(&uncatchableException{
 			err: v,