openresty / luajit2

OpenResty's Branch of LuaJIT 2
https://luajit.org/luajit.html
Other
1.23k stars 200 forks source link

unexpected index -1 comes up when copy a table with only one element #188

Open xiangnanscu opened 1 year ago

xiangnanscu commented 1 year ago

For me, this bug happens in my company computer resty command (Intel(R) Celeron(R) J4125 8GB ram win10x64 + wsl2 ubuntu 20.04+openresty 1.21.4 install from official apt)

It should be a luajit related bug because when I turn jit off for resty, there's no bug:

for ((i=1; i<=500; i++)); do resty -joff test.lua; done

BUG machine info:

设备名称 DESKTOP-S3UK088 处理器 Intel(R) Celeron(R) J4125 CPU @ 2.00GHz 2.00 GHz 机带 RAM 8.00 GB (7.85 GB 可用)

test.lua

local function jcopy(o)
  local function copy(v)
    if type(v) == "table" then
      local cv = {}
      for key, value in pairs(v) do
        if key == -1 then
          error(tostring(key))
        end
        cv[key] = copy(value)
      end
      return cv
    else
      return v
    end
  end

  return copy(o)
end

local function main()
  local n = 0
  for i = 1, 100, 1 do
    local c = jcopy { cond = { { "you shouldn't see this via index -1" } } }
    if c.cond[-1] then
      if n == 0 then
        -- print for the first time
        print(c.cond[-1][1])
      end
      n = n + 1
    end
  end
  if n > 0 then
    print("-1 index happends:" .. n)
  end
end

main()

then run test.lua 500 times

for ((i=1; i<=500; i++)); do resty test.lua; done

you will get output like:

root@DESKTOP-S3UK088:~/rsks# for ((i=1; i<=50; i++))
> do
>   resty test.lua
> done
you shouldn't see this via index -1
-1 index happends:77
you shouldn't see this via index -1
-1 index happends:29
you shouldn't see this via index -1
-1 index happends:8
you shouldn't see this via index -1
-1 index happends:6
you shouldn't see this via index -1
-1 index happends:38

But you shouldn't see any output if the copy work as expected. this is the minimal repo: https://github.com/xiangnanscu/luajit-bug

xiangnanscu commented 1 year ago

test envoriment: wsl2 Ubuntu 20.04

xiangnanscu commented 1 year ago

another strange thing is snippet

        if key == -1 then
          error(tostring(key))
        end

won't be triggered in the test. but the c.cond[-1] is accessible in the main function

xiangnanscu commented 1 year ago

This maybe a resty command related bug, because when I use luajit or lua directly, no bug:

for ((i=1; i<=50; i++)); do luajit test.lua; done
GUI commented 1 year ago

I can confirm I've observed the same behavior in OpenResty 1.21.4.1 where it was not an issue in OpenResty 1.19.9.1. So I'm not sure where the issue lies (luajit or some other OpenResty component), but something between those two versions seems to have introduced the issue.

In my case, I was testing an upgrade to OpenResty 1.21.4.1, but our web application's integration test suite started randomly failing due to unexpected JSON responses. The web app was returning JSON arrays with negative one indices that didn't match the expected results. I hadn't narrowed the issue down, but we do some table copying in the app, so it certainly seems like it could be this same issue. We've been blocked from upgrading to OpenResty 1.21.4.1 because of this issue, so thank you for finally shedding some light on the potential cause!

I can not reproduce this in an arm64 environment, but I can reproduce this in an amd64 environment. So it seems like the system architecture may also be in play.

I took your same test suite and was able to reproduce the issue within an OpenResty web app, just to better reflect how we were observing the issue with HTTP APIs. In order to easily reproduce this in the server environment, I had to turn lua_code_cache off, but our application's integration environment runs with lua_code_cache on, so it seems like it's maybe possible with both configurations, but turning it off make this easier to reproduce. So I'm not sure this really sheds any extra light, but just for completeness sake, here was my OpenResty HTTP reproduction around your test case:

test.conf:

lua_code_cache off;

server {
  listen 8888;
  location / {
    content_by_lua_block {
      local function jcopy(o)
        local function copy(v)
          if type(v) == "table" then
            local cv = {}
            for key, value in pairs(v) do
              if key == -1 then
                error(tostring(key))
              end
              cv[key] = copy(value)
            end
            return cv
          else
            return v
          end
        end

        return copy(o)
      end

      local function main()
        local n = 0
        for i = 1, 100, 1 do
          local c = jcopy { cond = { { "you shouldn't see this via index -1" } } }
          if c.cond[-1] then
            if n == 0 then
              -- print for the first time
              ngx.say(c.cond[-1][1])
              ngx.log(ngx.ERR, c.cond[-1][1])
            end
            n = n + 1
          end
        end
        if n > 0 then
          ngx.say("-1 index happends:" .. n)
          ngx.log(ngx.ERR, "-1 index happends:" .. n)
        end
      end

      main()
    }
  }
}

Run 1.19 and 1.21 servers separately:

docker run --rm -it -v "$(pwd)/test.conf":/etc/nginx/conf.d/default.conf -p 8819:8888 openresty/openresty:1.19.9.1-14-bullseye
docker run --rm -it -v "$(pwd)/test.conf":/etc/nginx/conf.d/default.conf -p 8821:8888 openresty/openresty:1.21.4.1-bullseye

Test against 1.19, no reported issues:

for ((i=1; i<=500; i++)); do curl http://localhost:8819/; done

Test against 1.21, various occurances reported:

for ((i=1; i<=500; i++)); do curl http://localhost:8821/; done
you shouldn't see this via index -1
-1 index happends:46
you shouldn't see this via index -1
-1 index happends:73
you shouldn't see this via index -1
-1 index happends:73
you shouldn't see this via index -1
-1 index happends:73
you shouldn't see this via index -1
-1 index happends:73
you shouldn't see this via index -1
-1 index happends:46
[...]
xiangnanscu commented 1 year ago

@GUI you'r welcome dude. I hope the openresty official could take this seriously because it's such a basic bug and let me feel the openresty is not reliable.

zhuizhuhaomeng commented 1 year ago

@xiangnanscu have you tried the lastest luajit from https://github.com/openresty/luajit2?

xiangnanscu commented 1 year ago

@zhuizhuhaomeng No, I haven't. As I say, luajit test.lua or lua test.lua won't trigger this bug in my machine. Have you confirmed this bug in your machine?

zhuizhuhaomeng commented 1 year ago

I can reproduce the bug with openresty 1.21.4. And can not reproduce the bug whit openresty built from source with the latest https://github.com/openresty/luajit2.

xiangnanscu commented 1 year ago

Oh, that's good, at lease one more dude join us. For me, this bug happens in my company computer (intel i3 8GB ram win10x64 + wsl2 ubuntu 20.04+openresty 1.21.4 install from official apt) , but not in my home computer (amd Ryzen 3600 32GB ram win10x64 + wsl2 ubuntu 20.04 +openresty 1.21.4 ).

Could you ask agentz for his opinion on this bug?