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

help request: file descriptor overflow when use request-id plugin nanoid algorithm #8931

Open
saiyan-qx opened this issue Feb 24, 2023 · 20 comments
Labels
bug Something isn't working

Comments

@saiyan-qx
Copy link

saiyan-qx commented Feb 24, 2023

Description

When i use request-id plugin nanoid algorithm,file descriptor increse to the limit of worker_rlimit_nofile : 20480,then cause the error : Too many open files。

I use lsof -p command, find a lot of file descriptors like this:
openresty 955 nfsnobody 45r CHR 1,9 0t0 71005690 /dev/urandom
openresty 955 nfsnobody 46r CHR 1,9 0t0 71005690 /dev/urandom
openresty 955 nfsnobody 47r CHR 1,9 0t0 71005690 /dev/urandom
openresty 955 nfsnobody 48r CHR 1,9 0t0 71005690 /dev/urandom
openresty 955 nfsnobody 49r CHR 1,9 0t0 71005690 /dev/urandom
openresty 955 nfsnobody 50r CHR 1,9 0t0 71005690 /dev/urandom

Thanks!

Environment

  • APISIX version 3.0
  • Operating system : Linux XXX 3.10.0-1160.80.1.el7.x86_64 change: added doc of how to load plugin. #1 SMP Tue Nov 8 15:48:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
  • OpenResty / Nginx version :nginx version: openresty/1.21.4.1
@tokers
Copy link
Contributor

tokers commented Feb 24, 2023

APISIX uses lua nanoid. It might be caused by the nanoid lib.

@saiyan-qx
Copy link
Author

saiyan-qx commented Feb 24, 2023

Eureka!The code of lua-resty-nanoid lib does not execute the close method. @tokers

source code

@tokers
Copy link
Contributor

tokers commented Feb 27, 2023

You may try to submit a PR for the lua-resty-nanoid repo, then let's upgrade the dependency in APISIX.

@liweitianux
Copy link
Contributor

Interesting, but I found the upstream lua-resty-nanoid project doesn't provide the "Issues" section...

What's more, the lua-resty-nanoid project simply imported the C code from nerdypepper/nanoid, and there have been an issue about the missing fclose().

@liweitianux
Copy link
Contributor

And I believe lua-resty-nanoid also has a memory leak issue. The ID buffer was allocated by malloc() but was never freed. It should be freed by the caller, i.e., the Lua C interface.

@liweitianux
Copy link
Contributor

On the other hand, I almost finished implementing a new Nano ID module for Lua (and C). I'll need to polish a bit more and try to get it onto LuaRocks. And I'll report back then.

See: https://github.com/liweitianux/nanoid

@spacewander spacewander added the bug Something isn't working label Mar 20, 2023
@shreemaan-abhishek
Copy link
Contributor

@liweitianux, any updates?

@liweitianux
Copy link
Contributor

Sorry. I've finished the nanoid code, but I haven't tried to publish it to LuaRocks yet. Will try it this weekend.

@shreemaan-abhishek
Copy link
Contributor

@liweitianux, looking forward to it.

@liweitianux
Copy link
Contributor

Hi @shreemaan-abhishek , so I've polished the nanoid code and submitted it to LuaRocks at https://luarocks.org/modules/liweitianux/lua-nanoid .

I've documented the module usage in the README at https://github.com/liweitianux/nanoid .

Please give it a try. Sorry for the delay.

I'd like it to be used in APISIX, which requires another MR. Let's see.

Cheers.

@liweitianux
Copy link
Contributor

And I believe lua-resty-nanoid also has a memory leak issue. The ID buffer was allocated by malloc() but was never freed. It should be freed by the caller, i.e., the Lua C interface.

Furthermore, that code doesn't NUL-terminate the C string, so I sometimes saw garbled code in the generated IDs... It's quite messy.

@shreemaan-abhishek
Copy link
Contributor

@liweitianux I get the following error when installing the luarock:

lua-nanoid 1.0.0-1 depends on lua >= 5.1 (5.1-1 provided by VM)
env MACOSX_DEPLOYMENT_TARGET=11.0 gcc -O2 -fPIC -I/opt/homebrew/opt/lua@5.1/include/lua5.1 -c nanoid_lua.c -o nanoid_lua.o -DNDEBUG
env MACOSX_DEPLOYMENT_TARGET=11.0 gcc -O2 -fPIC -I/opt/homebrew/opt/lua@5.1/include/lua5.1 -c nanoid.c -o nanoid.o -DNDEBUG
nanoid.c:90:9: error: call to undeclared function 'getrandom'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        getrandom(bytes, sizeof(bytes), 0);
        ^
1 error generated.

Error: Failed installing dependency: https://luarocks.org/lua-nanoid-1.0.0-1.rockspec - Build error: Failed compiling object nanoid.o
make: *** [deps] Error 1

@liweitianux
Copy link
Contributor

liweitianux commented Aug 4, 2023

Hi @shreemaan-abhishek , please report the issues regarding my lua-nanoid to its own project at https://github.com/liweitianux/nanoid .

The above issue is because getrandom() isn't available on macOS. Better to use arc4random(3) instead, but I need to add some compatibility code to support some old Linux versions, like CentOS 7 ...

@shreemaan-abhishek
Copy link
Contributor

Done!

@shreemaan-abhishek
Copy link
Contributor

@liweitianux, any updates?

@shreemaan-abhishek shreemaan-abhishek removed their assignment Sep 7, 2023
@Vacant2333
Copy link
Contributor

@liweitianux @shreemaan-abhishek if its done, we can close the issue now

@liweitianux
Copy link
Contributor

My nanoid module should work, although there is more optimization to improve the performance on old systems (e.g., CentOS 7).

@aikin-vip
Copy link
Contributor

sorry, I haven't paid attention to this place for a long time. I have read your previous discussion, and what I need to do now seems to be in two ways:

  1. use the @liweitianux code repo lua-nanoid and publish it to luarocks as nanoid-1.0.0-1. then modify this part of the main branch code of apisix.

    dependency change at :

    "nanoid = 0.1-1",

    it will be "nanoid = 1.0.0-1",

    generate method change at:

    return nanoid.safe_simple()

    it will be return nanoid.generate()

  2. The second way is to use the published module, lua-nanoid, and then modify the lua dependency of apisix.

    dependency version change at :

    "nanoid = 0.1-1",

    it will be "lua-nanoid = 1.1.1-1",

    generate method change at:

    return nanoid.safe_simple()

    it will be return nanoid.generate()

    local nanoid = require("nanoid")

    it will be local nanoid = require("lua-nanoid")

    which one do you suggest is better? @shreemaan-abhishek

    In addition, thanks @liweitianux 's contribution.

@liweitianux
Copy link
Contributor

Hi @aikin-vip,

local nanoid = require("nanoid")

it will be local nanoid = require("lua-nanoid")

This should be local nanoid = require("nanoid"), as documented in my README.

Cheers.

@aikin-vip
Copy link
Contributor

Hi @liweitianux
I have submitted the repair code and added dynamic parameters, waiting for merging 🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 📋 Backlog
Development

No branches or pull requests

7 participants