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

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

Open
xiangnanscu opened this issue Nov 10, 2022 · 9 comments
Open

Comments

@xiangnanscu
Copy link

xiangnanscu commented Nov 10, 2022

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

test envoriment: wsl2 Ubuntu 20.04

@xiangnanscu
Copy link
Author

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

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

GUI commented Dec 9, 2022

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

@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
Copy link
Contributor

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

@xiangnanscu
Copy link
Author

@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
Copy link
Contributor

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

xiangnanscu commented Dec 10, 2022

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?

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

No branches or pull requests

3 participants