luv icon indicating copy to clipboard operation
luv copied to clipboard

How callback errors are handled has changed (maybe unintentionally)

Open squeek502 opened this issue 6 years ago • 9 comments

Currently, any runtime errors within a callback don't affect the exit code of Lua (they are just printed to stderr and then execution continues). This seems to have changed in https://github.com/luvit/luv/commit/89f7bafac51706ba8a3280ee06b4225acddee3f9#diff-49ea526d495b77fdd140adde02ced70eL109 (https://github.com/luvit/luv/pull/254). Previously, any errors within a callback would lead to exit(-1) (the exit was added in https://github.com/luvit/luv/commit/7526c1d9955f085df9eb1c25fcc31c0cfe67e583).

Simple test case:

local uv = require('luv')

local timer = uv.new_timer()
uv.timer_start(timer, 10, 0, function()
  error('test')
end)

uv.run()

Before, running this would cause Lua to exit with status code -1, but right now it exits with status code 0.

This is a major change that might have been unintentional. If it wasn't, then we'll need to figure out the implications (since not exiting on error could lead to weird problems as mentioned in https://github.com/luvit/luv/commit/7526c1d9955f085df9eb1c25fcc31c0cfe67e583).

Related to #127, #128, #192.

@zhaozg

squeek502 avatar Oct 31 '19 22:10 squeek502

My original meaning is to avoid the exit of the program in the callback function, and I don't realize the existence of this problem (uncaught exceptions in event callbacks to avoid undefined behavior in libuv and luajit ). So, if possible, is there a way to achieve this, and if not, is it possible to revert to the exit mechanism when the error occurred?

Simply to say, I tend to restore the previous exit to avoid undefined behavior .

I'll make a PR to try.

zhaozg avatar Nov 01 '19 03:11 zhaozg

I think the problem is that there's no realistic way to recover from execution errors in callbacks, since Lua doesn't even have a way to know they happened. I'm away from my computer at the moment so I can't come up with an example, but I will tomorrow.

We can solve this one of two ways:

  • Add a method of error handling on the Lua side (this would be something like #127, #128, #192) so that users can decide what to do when a callback has an execution error if they have a way to recover from it
  • Exit immediately after an execution error in a callback (this would match how it worked previously)

squeek502 avatar Nov 01 '19 03:11 squeek502

@squeek502 A kind and responsible person. :thumbsup:

zhaozg avatar Nov 01 '19 04:11 zhaozg

Here's a somewhat contrived example, but I think it shows the problem.

local uv = require('luv')

local contents = nil

-- infinite timer that ends when contents is not nil
local timer = uv.new_timer()
timer:start(0, 500, function()
  print("timer tick")
  if contents ~= nil then
    print(contents)
    timer:stop()
    timer:close()
  end
end)

-- read a file async and set the contents var to the contents of the file
local fd = assert(uv.fs_open("something.txt", "r", tonumber('644', 8)))
assert(uv.fs_read(fd, 32, 0, function(err, chunk)
  assert(not err, err)
  -- uncomment the next line to force a simulated execution error
  --error("some execution error")
  contents = chunk
  assert(uv.fs_close(fd))
end))

uv.run()

This code works fine as long as the fs_read callback doesn't fail. The output will be (if 'hello' is the contents of something.txt):

timer tick
timer tick
hello

The program ends because the timer is closed and the uv loop is done.

However, if there is an execution error in the fs_read callback, then the timer/program will never end, and there's no realistic way to recover from this state, because there's no way for Lua to know that the callback failed (without a lot of custom protection code within every single callback).

When forcing an execution error with Luv 1.9.1 (which still had the hard exit), this is the output:

timer tick
lua: bad.lua:18: some execution error
stack traceback:
        [C]: in function 'error'
        bad.lua:18: in function <bad.lua:16>
        [C]: in function 'run'
        bad.lua:23: in main chunk
        [C]: ?

When forcing an execution error with current master (hard exit removed), this is the output:

timer tick
Uncaught Error: bad.lua:18: some execution error
stack traceback:
        [C]: in function 'error'
        bad.lua:18: in function <bad.lua:16>
        [C]: in function 'luv.run'
        bad.lua:23: in main chunk
        [C]: in ?
timer tick
timer tick
timer tick
timer tick
timer tick
timer tick
... (infinite loop that never ends)

So, the easy route is just to exit the program with an error status code whenever there's an execution error within a callback, since we can't predict if the error is recoverable or not. The harder route is allowing Lua to handle errors within callbacks (#127, #128, #192)

squeek502 avatar Nov 01 '19 22:11 squeek502

I guess this explains why my program will sometimes not exit after an error. I never thought much of it, only that maybe it was normal.

Why is lua_error not used? Because of potential pcalls?

SinisterRectus avatar Nov 01 '19 22:11 SinisterRectus

Why is lua_error not used? Because of potential pcalls?

Good question, I'm not sure.

squeek502 avatar Nov 01 '19 22:11 squeek502

I guess this explains why my program will sometimes not exit after an error. I never thought much of it, only that maybe it was normal.

Do you really want to exit after an error, we should try to avoid that but need more work to achive.

Why is lua_error not used? Because of potential pcalls?

We can try lua_error. and it's time to make a clear logic to handle errors.

zhaozg avatar Nov 02 '19 01:11 zhaozg

hope @bfredl to know this, this related with https://github.com/luvit/luv/pull/345, https://github.com/luvit/luv/pull/350. The default behavior of luv exit when luv callback occurs error. Please use luv_set_callback in https://github.com/luvit/luv/pull/350/files#diff-8792371db05ee9c0a8a1edb4d4f421acR592-R597 to make your custom c-callback.

zhaozg avatar Nov 03 '19 05:11 zhaozg

#345/#350 allows you to decide for yourself what happens on error. you could luv_set_callback an implementation which exit(1) on LUA_ERR* statuses.

bfredl avatar Nov 07 '19 16:11 bfredl