d5/tengo

Unrecoverable panic from VM when using (c *Compiled) RunContext

ganehag opened this issue · 4 comments

I recently had a panic: runtime error: integer divide by zero thrown by github.com/d5/tengo/v2.(*Compiled).RunContext. My immediate thought was to safeguard my code with a proper recovery clause in a defer statement. Which I did, only to discover that the issue persisted.

panic: runtime error: integer divide by zero

goroutine 11 [running]:
github.com/d5/tengo/v2.(*Int).BinaryOp(0xc000025838, 0xe, {0x84bd18, 0xc000025868})
../github.com/d5/tengo/v2/objects.go:1030 +0xac5
github.com/d5/tengo/v2.(*VM).run(0xc0001e2000)
../github.com/d5/tengo/v2/vm.go:116 +0x2b8c
github.com/d5/tengo/v2.(*VM).Run(0xc0001e2000)
../github.com/d5/tengo/v2/vm.go:77 +0xd2
github.com/d5/tengo/v2.(*Compiled).RunContext.func1()
../github.com/d5/tengo/v2/script.go:222 +0x26
created by github.com/d5/tengo/v2.(*Compiled).RunContext
../github.com/d5/tengo/v2/script.go:221 +0x2c9

Upon investigating the code in github.com/d5/tengo/v2/script.go:221, I realised that a Go function inside RunContext executed the VM. Unfortunately, this Go function was not recovering from the panic thrown by the VM, and my code can't recover from it due to it being raised in a Go function.

Patching the code in github.com/d5/tengo/v2/script.go:221 so that the Go function uses a defer statement, returning the recovered error from RunContext to the caller;

// github.com/d5/tengo/v2/script.go:221
go func() {
	defer func() {
		if r, ok := recover().(error); ok {
			ch <- r
		}
	}()
	ch <- v.Run()
}()

Now, I am not sure if this is the proper way to do this. So someone with a more profound knowledge of Go-ism has to weigh in here.

If this is an acceptable solution, then maybe a similar defer statement should be added to (c *Compiled) Run() on L206.

d5 commented

Not entirely sure what you expect, but if your solution is to recover at Compiled.Run, you should be able to achieve the same goal with defer with recover inside the function that calls Compiled.Run. Also if your code throws divide by zero, you should fix that one first no matter what.

I am talking about (c *Compiled).RunContext and not (*Compiled).Run. It differs from RunContext in that it can easily be managed by doing an ordinary "defer and recover" in the function from which I execute (c *Compiled).Run. But (c *Compiled).RunContext has an internal go routine to execute the VM in the background as a way to be able to handle the Context event. When any panic is caused by the VM this go routine will crash and I can't recover from that. Not in any way that I know of other than having the go routine inside of (c *Compiled).RunContext taking care of the panic.

d5 commented

Sorry I misread your question. My bad. And yes I think your solution looks good. Feel free to open a PR. I will take a look.

Yes, will do.