Skip to content

Commit

Permalink
Don't clear interrupt until the stack is empty (dop251#405)
Browse files Browse the repository at this point in the history
* 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]>
(cherry picked from commit 8795259)
  • Loading branch information
mstoykov authored and Gabri3l committed Sep 1, 2022
1 parent 4311cf2 commit 21a34e5
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 3 deletions.
16 changes: 15 additions & 1 deletion runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
115 changes: 115 additions & 0 deletions runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (;;) {}})")
Expand Down
2 changes: 0 additions & 2 deletions vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 21a34e5

Please sign in to comment.