Skip to content

Commit

Permalink
fix(pdk) ensure 'kong.ctx.plugin' is light-thread safe
Browse files Browse the repository at this point in the history
* Store namespace keys in `ngx.ctx` to ensure all dynamically generated
  namespaces are isolated on a per-request basis.
* Introduce a new global API (which is a private API for use by core
  only) to explicitly delete a namespace.

A note on the benefits of this new implementation:

* By using a table to keep track of namespaces in `ngx.ctx`, we ensure
  that users of `ngx.ctx` cannot access the table namespaces, thus
  properly isolating it and avoiding polluting `ngx.ctx` for core and/or
  plugins. We can think of it as a double-pointer reference to the
  namespace. Each request gets a pre-allocated table for 4 namespaces
  (of course not limited to 4), with the assumption that most instances
  do not execute more than 4 plugins using `kong.ctx.plugin` at a time.
* We ensure that namespace key references cannot be `nil` to ensure a
  safe usage from the core.
* All namespace key references are still weak when assigned to a
  namespace, thus tying the lifetime of the namespace to that of its key
  reference. Similarly to the previous implementation, this is done to
  ensure we avoid potential memory leaks. That said, the
  `kong.ctx.plugin` namespace does use the plugin's conf for that
  purpose anymore (See 40dc146), which
  alleviates such concerns in the current usage of this API.
* All tables allocated for namespace key references and namespaces keys
  themselves will be released when `ngx.ctx` will be GC'ed.
* We also ensure than `kong.ctx.*` returns `nil` when `ngx.ctx` is not
  supported ("init" phase).

Co-Authored-By: tdelaune <[email protected]>

Fix #4379
Fix #5853
See #5851
See #5459
  • Loading branch information
thibaultcha committed May 14, 2020
1 parent b441f4c commit 7694bdb
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 12 deletions.
27 changes: 26 additions & 1 deletion kong/global.lua
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,36 @@ function _GLOBAL.set_named_ctx(self, name, key)
error("name cannot be an empty string", 2)
end

if key == nil then
error("key cannot be nil", 2)
end

if not self.ctx then
error("ctx PDK module not initialized", 2)
end

self.ctx.__set_namespace(name, key)
end


function _GLOBAL.del_named_ctx(self, name)
if not self then
error("arg #1 cannot be nil", 2)
end

if type(name) ~= "string" then
error("name must be a string", 2)
end

if #name == 0 then
error("name cannot be an empty string", 2)
end

if not self.ctx then
error("ctx PDK module not initialized", 2)
end

self.ctx.keys[name] = key
self.ctx.__del_namespace(name)
end


Expand Down
61 changes: 53 additions & 8 deletions kong/pdk/ctx.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@


local ngx = ngx
local ngx_get_phase = ngx.get_phase


-- shared between all global instances
local _CTX_SHARED_KEY = {}
local _CTX_CORE_KEY = {}


-- dynamic namespaces, also shared between global instances
local _CTX_NAMESPACES_KEY = {}


---
-- A table that has the lifetime of the current request and is shared between
-- all plugins. It can be used to share data between several plugins in a given
Expand Down Expand Up @@ -86,19 +91,58 @@ local _CTX_CORE_KEY = {}
--
-- kong.log(value) -- "hello world"
-- end
local function new(self)
local _CTX = {
-- those would be visible on the *.ctx namespace for now
-- TODO: hide them in a private table shared between this
-- module and the global.lua one
keys = setmetatable({}, { __mode = "v" }),
}


local function new(self)
local _CTX = {}
local _ctx_mt = {}
local _ns_mt = { __mode = "v" }


local function get_namespaces(nctx)
local namespaces = nctx[_CTX_NAMESPACES_KEY]
if not namespaces then
-- 4 namespaces for request, i.e. ~4 plugins
namespaces = self.table.new(0, 4)
nctx[_CTX_NAMESPACES_KEY] = setmetatable(namespaces, _ns_mt)
end

return namespaces
end


local function set_namespace(namespace, namespace_key)
local nctx = ngx.ctx
local namespaces = get_namespaces(nctx)

local ns = namespaces[namespace]
if ns and ns == namespace_key then
return
end

namespaces[namespace] = namespace_key
end


local function del_namespace(namespace)
local nctx = ngx.ctx
local namespaces = get_namespaces(nctx)
namespaces[namespace] = nil
end


function _ctx_mt.__index(t, k)
if k == "__set_namespace" then
return set_namespace

elseif k == "__del_namespace" then
return del_namespace
end

if ngx_get_phase() == "init" then
return
end

local nctx = ngx.ctx
local key

Expand All @@ -109,7 +153,8 @@ local function new(self)
key = _CTX_SHARED_KEY

else
key = t.keys[k]
local namespaces = get_namespaces(nctx)
key = namespaces[k]
end

if key then
Expand Down
1 change: 0 additions & 1 deletion t/01-pdk/05-client/09-load-consumer.t
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,3 @@ GET /t
cannot load a consumer with an id that is not a uuid
--- no_error_log
[error]
65 changes: 63 additions & 2 deletions t/02-global/02-set-named-ctx.t
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ marry
=== TEST 3: set_named_ctx() arbitrary namespaces can be discarded
=== TEST 3: set_named_ctx() arbitrary namespaces can be discarded via del_named_ctx()
--- http_config eval: $t::Util::HttpConfig
--- config
location = /t {
Expand All @@ -93,7 +93,7 @@ marry
kong.ctx.custom.cats = "marry"
ngx.say(kong.ctx.custom.cats)
kong_global.set_named_ctx(kong, "custom", nil)
kong_global.del_named_ctx(kong, "custom")
ngx.say(kong.ctx.custom)
}
}
Expand Down Expand Up @@ -181,3 +181,64 @@ GET /t
ctx PDK module not initialized
--- no_error_log
[error]
=== TEST 7: set_named_ctx() arbitrary namespaces are light-thread safe
--- http_config
init_by_lua_block {
kong_global = require "kong.global"
kong = kong_global.new()
kong_global.init_pdk(kong)
}
--- config
location = /lua {
content_by_lua_block {
local args = assert(ngx.req.get_uri_args())
local thread_name = args.name
local value = args.val
kong_global.set_named_ctx(kong, "thread", thread_name)
kong.ctx.thread.val = value
ngx.sleep(0.1)
ngx.say("thread ", thread_name, ": ", kong.ctx.thread.val)
}
}
location = /t {
echo_subrequest_async GET '/lua' -q 'name=A&val=hello';
echo_subrequest_async GET '/lua' -q 'name=B&val=world';
}
--- request
GET /t
--- response_body
thread A: hello
thread B: world
--- no_error_log
[error]
=== TEST 8: set_named_ctx() arbitrary namespaces invalid argument 'key'
--- http_config eval: $t::Util::HttpConfig
--- config
location = /t {
content_by_lua_block {
local kong_global = require "kong.global"
local kong = kong_global.new()
kong_global.init_pdk(kong)
local pok, perr = pcall(kong_global.set_named_ctx, kong, "custom", nil)
if not pok then
ngx.say(perr)
end
}
}
--- request
GET /t
--- response_body
key cannot be nil
--- no_error_log
[error]

0 comments on commit 7694bdb

Please sign in to comment.