luvit / luv

Bare libuv bindings for lua
Apache License 2.0
831 stars 187 forks source link

V1.45 #649

Closed zhaozg closed 1 year ago

zhaozg commented 1 year ago

close https://github.com/luvit/luv/issues/646

squeek502 commented 1 year ago

My suggestion would be to remove the update lua to v5.4.6 and update lua-compat-5.3 to HEAD commits from this branch to see if updating Lua is the cause of the Valgrind errors. If it is, then we should just do that separately from updating Libuv.

See https://github.com/luvit/luv/issues/646#issuecomment-1555730462

zhaozg commented 1 year ago

Ok, change finished.

squeek502 commented 1 year ago

Created a pull request with the remaining stuff: https://github.com/zhaozg/luv/pull/5

zhaozg commented 1 year ago

Good jobs, Let wait appveyor report status.

zhaozg commented 1 year ago

failed https://ci.appveyor.com/project/racker-buildbot/luv/builds/47095901

ok 58 misc - uv.version and uv.version_string
  .\tests/test-misc.lua:31: assertion failed!
  stack traceback:
    [C]: in function 'assert'
    .\tests/test-misc.lua:31: in function 'fn'
    .\lib/tap.lua:59: in function <.\lib/tap.lua:48>
    [C]: in function 'xpcall'
    .\lib/tap.lua:48: in function 'run'
    .\lib/tap.lua:146: in function 'tap'
    tests\run.lua:23: in main chunk
    [C]: at 0x01205430
not ok 59 misc - memory size
squeek502 commented 1 year ago

Strange, will figure that out in a bit. I'm currently on Linux reporting the valgrind libuv stuff :smile:

zhaozg commented 1 year ago

Strange, will figure that out in a bit. I'm currently on Linux reporting the valgrind libuv stuff 😄

Maybe lua cast it as signed number on 32 bits arch. I do some check.

squeek502 commented 1 year ago

Looks like you're right. Changing luv_get_available_memory to use lua_pushnumber should fix it I think.

squeek502 commented 1 year ago

Looks like pushunsigned won't work, since the lua-compat-5.3 implementation just casts to lua_Integer anyway:

https://github.com/lunarmodules/lua-compat-5.3/blob/8f8e4c6adb43e107f5902e784ef207dc3c8ca06b/c-api/compat-5.3.h#L258-L259

I think lua_pushnumber is okay, it's what luv_get_free_memory, luv_get_total_memory, etc use.

zhaozg commented 1 year ago

ok, I will back in 20 minutes, outsides now.

squeek502 commented 1 year ago

https://github.com/zhaozg/luv/pull/6