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

Using outer scope required modules inside an ngx.timer.at handler #2312

Open
Maor-Magori-Forter opened this issue May 20, 2024 · 4 comments
Open

Comments

@Maor-Magori-Forter
Copy link

Maor-Magori-Forter commented May 20, 2024

Hello,
I've encountered an error which I understand the source to but unsure what's the best practice here.
I have the following example module respond.lua:

local ngx = require 'ngx'
local config = require 'config'

local required_module = require 'module_with_function'

local function handler(_, arg)
   required_module.fn(arg)
end

local function main()
    ngx.timer.at(0, handler, 'test')

    ngx.status = config.code
    ngx.say(config.response)
    ngx.exit(config.code)
end

main()

This would result in attempt to index upvalue 'fn' error thrown from the handler function.
I deduced this is happening because the ngx.exit is called before the call to fn but this context sharing is unintuitive.
If I add a simple sleep before the call to ngx.exit it works just fine meaning the handler function does share the same context as the encapsulating module.

I resorted to adding another require inside the handler function but I would appreciate if there was a clear-cut answer on whether or not modules defined outside the ngx.timer.at handler function are available in it.

For clarification, lua_package_path is set up correctly.

OpenResty: version 1.25.3.1-2

@Rockybilly
Copy link

Hey, the problem seems like the lifetime of request context variables. But the problem arises only when you try to call the name required_module within handler, and required_module no longer exists (request memory has been freed). A delay hides this problem, but an actual solution would be one of these.

local function handler(_, mod, arg)
  mod.fn(arg)
end

local function main()
  ngx.timer.at(0, handler, required_module, 'test')
end

Or

local function handler(_, func, arg)
  func(arg)
end

local function main()
  ngx.timer.at(0, handler, required_module.fn, 'test')
end

You pass the module or the function as a parameter to the handler. So it no longer requires to reference it from the context of the request.

@Maor-Magori-Forter
Copy link
Author

Hey @Rockybilly Thanks for the quick response!

I'm not sure if LuaJIT passes by value or reference so I'm assuming "by reference" here. Won't the passed function get the same treatment as calling the function from within the handler and get removed once the request finishes?
And is there any harm in requiring the module again in the handler function?

Any chance you could point me to the code that frees up the memory on ngx.exit?

Appreciate your help here 🙏

@Rockybilly
Copy link

Rockybilly commented May 21, 2024

To your first question, the contributers are probably better equipped to answer. However my guess is, a garbage collector would be reference counting the references before freeing them. So when a timer is set to run, the function body has never been run when the request context is freed, so no reference count to hold on to the value. But when you use it in function parameters, the reference count would be increased and it would not be freed until function is done running and the parameter goes out of scope. I can at least say that I tried it and didn't receive any errors.

To your second question, zero harm in requiring the module in the function. If performance and micro-optimization is one of your concerns, it would mean one extra local variable declaration, function call and table lookup (require() accesses package.loaded). But probably creating a timer itself has already much more overhead than these, so you need not consider these.

For your third question, ngx.exit does not itself free all the memory, rather probably lua garbage collectors and Nginx freeing request context from the memory pool. So it would be hard for me to pinpoint any part of code that literally frees that object. This part I don't know well enough to answer.

However, for testing, you can set your timer for like for 1 second delay, guarantees that the request context is gone for sure. And try something like ngx.log(ngx.CRIT, type(any_object)), and check if any object still exists when this code runs and prints the value in the Nginx error log.

@Maor-Magori-Forter
Copy link
Author

I appreciate your help here @Rockybilly.
I'll stick currently to having an additional requires in the handler functions to have some segregation between the "threads".

I do see the behavior of referencing an outer scope variable inside the handler function without passing them as arguments, in other projects. I assume everything works for them since the request context gets freed long after the handler finishes.

I would appreciate an answer from one of the contributors on what's the recommended practice here if one of them stumbles upon this issue

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

2 participants