interp: recover in Eval and EvalPath

[Bai-Yingjie]

Now Eval() and EvalPath() recover from internal panic and return it as error with call stacks.

This fixes the case where user code calls os.Exit(1), which is converted to panic() and breaks down the program.

[mvertes]

Can you provide a test program which fails before and works after your change request?

I'm not able to reproduce the issue you mention. We already have the panic wrapped in an error in the current code as far as I see.

[Bai-Yingjie]

Thanks for your review, and sorry for the late response...

To preproduce the panic, put below code to a file in a directory, e.g. ./test/hello.go:

package main

import (
        "fmt"
        "os"
)

func main() {
        fmt.Println("Hello, playground")
        os.Exit(1)
}

Then run yaegi with the directory can reproduce the panic:

$ ./yaegi ./test
Hello, playground
test/hello.go:9:2: panic
panic: os.Exit(1) [recovered]
        panic: os.Exit(1)

goroutine 1 [running]:
github.com/traefik/yaegi/interp.runCfg.func1()
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:205 +0x12e
panic({0xd47640, 0xc00007e970})
        /usr/local/go/src/runtime/panic.go:838 +0x207
github.com/traefik/yaegi/stdlib.osExit(0x0?)
        /repo/yingjieb/3rdparty/yaegi/stdlib/restricted.go:14 +0x59
reflect.Value.call({0xd45a80?, 0xecb318?, 0x410225?}, {0xe649b0, 0x4}, {0xc00000c678, 0x1, 0x4cbc05?})
        /usr/local/go/src/reflect/value.go:556 +0x845
reflect.Value.Call({0xd45a80?, 0xecb318?, 0x453452?}, {0xc00000c678, 0x1, 0x1})
        /usr/local/go/src/reflect/value.go:339 +0xbf
github.com/traefik/yaegi/interp.callBin.func2({0xd45a80?, 0xecb318?, 0xc000245620?}, {0xc00000c678?, 0xc00007e910?, 0xc000245638?})
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:1453 +0x28
github.com/traefik/yaegi/interp.callBin.func11(0xc00037c160)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:1641 +0x15f
github.com/traefik/yaegi/interp.runCfg(0xc0003afa40, 0xc00037c160, 0x0?, 0xc0002457f8?)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:213 +0x29d
github.com/traefik/yaegi/interp.(*Interpreter).run(0xc000032480, 0xc0003aef00, 0xc00037c000?)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:119 +0x374
github.com/traefik/yaegi/interp.(*Interpreter).importSrc(0xc000032480, {0xe650cc, 0x4}, {0x7ffe66ad0c0e, 0x6}, 0x1)
        /repo/yingjieb/3rdparty/yaegi/interp/src.go:170 +0xb55
github.com/traefik/yaegi/interp.(*Interpreter).EvalPath(0xc000032480, {0x7ffe66ad0c0e, 0x6})
        /repo/yingjieb/3rdparty/yaegi/interp/interp.go:509 +0xdb
main.run({0xc0001a8010?, 0x1, 0x1})
        /repo/yingjieb/3rdparty/yaegi/cmd/yaegi/run.go:118 +0xc0b
main.main()
        /repo/yingjieb/3rdparty/yaegi/cmd/yaegi/yaegi.go:144 +0x2dc

When executing a directory, EvalPath() is used:

EvalPath()
    interp.importSrc()

interp.importSrc() compiles all the files in the directory and calls interp.run() to run the main() function. There is a defer recover deep in the function runCfg(), but it re-triggers the panic:

The top level EvalPath() does not defer & recover the panic, which caused the whole process to die immediately.

On the other hand, when executing a file, although the program also exits, but it does not die because of panic, but because the panic was recovered in interp.Execute() so the program can exit normally.

Eval()
    interp.compileSrc()
    interp.Execute()

It prints the call stack similar with the above one, but it is not a direct result of panic, it is just the returned error of Eval()

$ ./yaegi ./test/hello.go
Hello, playground
./test/hello.go:9:2: panic
run: os.Exit(1)
goroutine 1 [running]:
runtime/debug.Stack()
        /usr/local/go/src/runtime/debug/stack.go:24 +0x65
github.com/traefik/yaegi/interp.(*Interpreter).Execute.func1()
        /repo/yingjieb/3rdparty/yaegi/interp/program.go:146 +0x94
panic({0xd47640, 0xc00011a920})
        /usr/local/go/src/runtime/panic.go:838 +0x207
github.com/traefik/yaegi/interp.runCfg.func1()
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:205 +0x12e
panic({0xd47640, 0xc00011a920})
        /usr/local/go/src/runtime/panic.go:838 +0x207
github.com/traefik/yaegi/stdlib.osExit(0x0?)
        /repo/yingjieb/3rdparty/yaegi/stdlib/restricted.go:14 +0x59
reflect.Value.call({0xd45a80?, 0xecb318?, 0x410225?}, {0xe649b0, 0x4}, {0xc000126630, 0x1, 0x4cbc05?})
        /usr/local/go/src/reflect/value.go:556 +0x845
reflect.Value.Call({0xd45a80?, 0xecb318?, 0x453452?}, {0xc000126630, 0x1, 0x1})
        /usr/local/go/src/reflect/value.go:339 +0xbf
github.com/traefik/yaegi/interp.callBin.func2({0xd45a80?, 0xecb318?, 0xc0001cf8c8?}, {0xc000126630?, 0xc00011a8c0?, 0xc0001cf8e0?})
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:1453 +0x28
github.com/traefik/yaegi/interp.callBin.func11(0xc00037c160)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:1641 +0x15f
github.com/traefik/yaegi/interp.runCfg(0xc0003afa40, 0xc00037c160, 0x0?, 0x0?)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:213 +0x29d
github.com/traefik/yaegi/interp.(*Interpreter).run(0xc00037a240, 0xc0003aef00, 0xc00037c000?)
        /repo/yingjieb/3rdparty/yaegi/interp/run.go:119 +0x374
github.com/traefik/yaegi/interp.(*Interpreter).Execute(0xc00037a240, 0xc000457650)
        /repo/yingjieb/3rdparty/yaegi/interp/program.go:172 +0x24b
github.com/traefik/yaegi/interp.(*Interpreter).eval(0xc00037a240, {0xc0002dcab0?, 0x85?}, {0x7fff02a91c05?, 0xc0002dc990?}, 0x85?)
        /repo/yingjieb/3rdparty/yaegi/interp/interp.go:568 +0x5c
github.com/traefik/yaegi/interp.(*Interpreter).EvalPath(0xc00037a240, {0x7fff02a91c05, 0xf})
        /repo/yingjieb/3rdparty/yaegi/interp/interp.go:517 +0xab
main.runFile(0x7fff02a91c05?, {0x7fff02a91c05, 0xf}, 0x0)
        /repo/yingjieb/3rdparty/yaegi/cmd/yaegi/run.go:153 +0xee
main.run({0xc000030050?, 0x1, 0x1})
        /repo/yingjieb/3rdparty/yaegi/cmd/yaegi/run.go:116 +0xbec
main.main()
        /repo/yingjieb/3rdparty/yaegi/cmd/yaegi/yaegi.go:144 +0x2dc

In a program like gshell, we use yaegi to execute go source codes inside a goroutine, it is better that the Eval() and EvalPath() never panics directly, but always recovers the panic at top level and returns the error, so that the whole program will not break because of panic.

[Bai-Yingjie]

@mvertes Is there any other steps should I do to deliver the patch? I see all checks have passed but review approval is required.