mattn / anko

Scriptable interpreter written in golang
http://play-anko.appspot.com/
MIT License
1.45k stars 120 forks source link

goroutine crash silently #328

Closed alaingilbert closed 4 years ago

alaingilbert commented 4 years ago

Any idea how we could make a crash in a goroutine stop the VM ? eg:

time = import("time")
for {
  go func() {
    println("before")
    This should stop the VM
    println("will never print")
  }()
  time.Sleep(time.Second)
}

This program will not crash and will just keep running as if everything is fine. It would be nice to stop the VM in such case, and get back the error.

My workaround right now is to define a Go function

ctx, cancel := context.WithCancel(context.Background())

_ = myvm.Define("Go", func(fn func()) {
  go func() {
    defer func() {
      if r := recover(); r != nil {
        println(r)
        cancel()
      }
    }()
    fn()
  }()
})

myscript := `
time = import("time")
for {
  Go(func() {
    println("before")
    This will stop the VM
    println("will never print")
  })
  time.Sleep(time.Second)
}`

myvm.ExecuteContext(ctx, myscript)

But I have to edit the original anko code to prevent the use of the go keyword.

MichaelS11 commented 4 years ago

The below code gives me a syntax error.

time = import("time")
for (i = 0; i < 10; i++) {
    go func() {
        println("before")
        This should stop the VM
        println("will never print")
    }()
    time.Sleep(time.Second)
}
alaingilbert commented 4 years ago

Must be something that was fixed in more recent version.

But here is another example that is broken in the current playground: This one will stop the VM with a undefined symbol 'a' error. http://play-anko.appspot.com/p/cafefad589d3b20b61b8055c487ef64c03aad32d VS This one works just fine, even though it uses undefined symbol (and that the goroutine crash) http://play-anko.appspot.com/p/a6bca82496c93aa8b42790e21bc07b57bb681180

MichaelS11 commented 4 years ago

For me, this:

time = import("time")
for i = 0; i < 3; i++ {
    go func() {
        println("before")
        println("will never print")
    }()
    time.Sleep(time.Second)
    println(a.b.c)
}
println("done")

Outputs:

before
will never print
2020/03/02 14:16:02 Execute error: undefined symbol 'a'

The playground may not be an up to date version.

alaingilbert commented 4 years ago

This is the correct output.

The second one is

before
before
before
done

(while it should stop after the first before)

MichaelS11 commented 4 years ago

For me, this:

time = import("time")
for i = 0; i < 3; i++ {
    go func() {
        println("before")
        println(a.b.c)
        println("will never print")
    }()
    time.Sleep(time.Second)
}
println("done")

Also outputs:

before
before
before
done

Which is correct. Anko will not crash/stop on go function error.

MichaelS11 commented 4 years ago

Let's stick with go func for the moment, can look at try next.

I am not sure what you mean about looping forever, the go func stops after the error.

Example:

time = import("time")
go func() {
    for i = 0; i < 3; i++ {
        println("a1")
        println(a)
        println("a2")
    }
}()
time.Sleep(time.Second)
println("done")

Output:

a1
done
alaingilbert commented 4 years ago

Sorry I deleted my comment after a few seconds, seeing the example wasn't good.

Indeed, it doesn't stop. But it should stop.
This is invalid code.
And the goroutine actually crash.
And you will never know it crashed.
This is just as good as a `try catch ignore` (which is no good at all).
Let me show you how bad this could be:

\`\`\`go
go func() {
    for {
        println("I'm going to do something every second")
        doSomething()
        time.Sleep(time.Second)
    }
}()
<-WaitForeverChan
\`\`\`
This is supposed to do something every second, but instead, it will fill your hard drive with useless logs at an insane rate because it will loop forever without executing the Sleep.
alaingilbert commented 4 years ago

but in any way, you see that the script is supposed to do something, and it won't do anything as the thread is dead, and you get no feedback whatsoever.

MichaelS11 commented 4 years ago

As I said, Anko will not crash/stop on go func error.

MichaelS11 commented 4 years ago

@alaingilbert So can you close this? Or is there something else?

alaingilbert commented 4 years ago

This is major flaw if you ask me. But sure, feel free to just close it if you don't plan to improve it.

MichaelS11 commented 4 years ago

All depends on how you look at it. Can also look at it a major feature. People may like it so the whole program does not crash. Plus if you do want the whole VM to end, you have shown it can be done by using a function.

alaingilbert commented 4 years ago

So you're telling me that people want their goroutine to just crash and give no feedback ? I would expect the same kind of behavior on the main thread. I think the app should never ever crash, and never return an error at all.

alaingilbert commented 4 years ago

Alright I'll stop here. This is not sane for any of us. Thanks for the good work you're doing.