Recursive call crashes gpython
xiaxinmeng opened this issue · 3 comments
We define a function and recursive call the function in test.py crashing gpython. This case cannot crash CPython and it reports a RecursionError.
test.py:
def f():
f()
f()
Expected output: RecursionError
Actual output:
runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow
runtime stack:
runtime.throw(0x66c609, 0xe)
/opt/go/go1.11/src/runtime/panic.go:608 +0x72
runtime.newstack()
/opt/go/go1.11/src/runtime/stack.go:1008 +0x729
runtime.morestack()
/opt/go/go1.11/src/runtime/asm_amd64.s:429 +0x8f
goroutine 1 [running]:
runtime.heapBitsSetType(0xc0027a3440, 0x30, 0x30, 0x658180)
/opt/go/go1.11/src/runtime/mbitmap.go:911 +0xa30 fp=0xc0226e2390 sp=0xc0226e2388 pc=0x416c80
runtime.mallocgc(0x30, 0x658180, 0x1, 0x0)
/home/ncw/go/src/github.com/go-python/gpython/vm/eval.go:1428 +0x40 fp=0xc0226e83c8 sp=0xc0226e8378 pc=0x5b33d0
...additional frames elided...
.......................
goroutine 19 [syscall]:
os/signal.signal_recv(0x0)
/opt/go/go1.11/src/runtime/sigqueue.go:139 +0x9c
os/signal.loop()
/opt/go/go1.11/src/os/signal/signal_unix.go:23 +0x22
created by os/signal.init.0
/opt/go/go1.11/src/os/signal/signal_unix.go:29 +0x41
version: Gpython3.4.0 on Ubuntu 16.04
A couple of way to fix this:
- recover from the panic and generate the RecursionError exception
- implement the recursion check in Vm.Call, similar to CPython (add a counter for pending calls in the Vm struct, increment/decrement in Vm.Call and generate the RecursionError when the limit is reached).
The 2nd method would probably be better since CPython has a limit of 1000 nested calls (by default) while gpython runs until it runs out of memory.
Out of curiosity, this is a lot of effort for an issue that seems a second place to many other areas of potential improvement for gpython. Perhaps there something I'm missing? I do agree it's not helpful to be able to run python in gpython that induces a crash, but there are many things that may do that.
If @sbinet is interested, I can work on a PR for this if we agree this is worth adding.