google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.26k stars 204 forks source link

starlark: use defer for cleanup so that panics pass through safely #422

Closed adonovan closed 1 year ago

adonovan commented 1 year ago

This change uses defer for the cleanup logic in in Call and Function.CallInternal so that panics originating in application functions called by the interpreter can safely pass through the interpreter without leaving the thread in a bad state (e.g. frames still on the stack).

This seems to have an impact of ~0.7% on the 'calling' benchmark, which represents the worst possible case.

Also, a test.

Fixes #419

vanishs commented 1 year ago

I've been using this branch for over a week and it works just fine.

In my program, the built-in functions I provided would use the panic trick to implement some functionality. but until this fix, the panic trick would leave the starlark stack in a wrong state.

I hope this branch will be merged soon. thanks.

vanishs commented 1 year ago

@brandjon hi. could you take a few time to do a review?