Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How callback errors are handled has changed (maybe unintentionally) #433

Open
squeek502 opened this issue Oct 31, 2019 · 9 comments
Open

Comments

@squeek502
Copy link
Member

squeek502 commented Oct 31, 2019

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 89f7baf#diff-49ea526d495b77fdd140adde02ced70eL109 (#254). Previously, any errors within a callback would lead to exit(-1) (the exit was added in 7526c1d).

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 7526c1d).

Related to #127, #128, #192.

@zhaozg

@squeek502 squeek502 changed the title Maybe unintentional change in how errors are handled How callback errors are handled has changed (maybe unintentionally) Oct 31, 2019
@zhaozg
Copy link
Member

zhaozg commented Nov 1, 2019

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.

@squeek502
Copy link
Member Author

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:

zhaozg added a commit to zhaozg/luv that referenced this issue Nov 1, 2019
1. To avoid undefine…d behavior in libuv and luajit follow luvit@7526c1d.
2. Replay luvit#433.
3. LUVF_CALLBACK_ flags introduced by luvit@5b68007#diff-4221738e9478598ca8f06be66d888559
@zhaozg
Copy link
Member

zhaozg commented Nov 1, 2019

@squeek502 A kind and responsible person. 👍

@squeek502
Copy link
Member Author

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)

@SinisterRectus
Copy link
Member

SinisterRectus commented Nov 1, 2019

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?

@squeek502
Copy link
Member Author

squeek502 commented Nov 1, 2019

Why is lua_error not used? Because of potential pcalls?

Good question, I'm not sure.

@zhaozg
Copy link
Member

zhaozg commented Nov 2, 2019

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
Copy link
Member

zhaozg commented Nov 3, 2019

hope @bfredl to know this, this related with #345, #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.

@bfredl
Copy link

bfredl commented Nov 7, 2019

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants