Skip to content

Commit

Permalink
fix(mlcache): release lock upon exiting the renew function (#12743)
Browse files Browse the repository at this point in the history
Co-authored-by: Qi <add_sp@outlook.com>
  • Loading branch information
chobits and ADD-SP authored Mar 21, 2024
1 parent 9793768 commit b5f1da0
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 11 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/kong/fix-mlcache-renew-lock-leaks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Fixed an issue that leaking locks in the internal caching logic
type: bugfix
scope: Core
30 changes: 19 additions & 11 deletions kong/resty/mlcache/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -826,14 +826,15 @@ function _M:renew(key, opts, cb, ...)

local version_before = get_version(shmerr or 0)

local lock, err = resty_lock:new(self.shm_locks, self.resty_lock_opts)
local lock, lock_err = resty_lock:new(self.shm_locks, self.resty_lock_opts)
if not lock then
return nil, "could not create lock: " .. err
return nil, "could not create lock: " .. lock_err
end

local elapsed, err = lock:lock(LOCK_KEY_PREFIX .. namespaced_key)
if not elapsed and err ~= "timeout" then
return nil, "could not acquire callback lock: " .. err
local elapsed
elapsed, lock_err = lock:lock(LOCK_KEY_PREFIX .. namespaced_key)
if not elapsed and lock_err ~= "timeout" then
return nil, "could not acquire callback lock: " .. lock_err
end

local is_hit
Expand All @@ -847,7 +848,11 @@ function _M:renew(key, opts, cb, ...)
if shmerr then
-- shmerr can be 'flags' upon successful get_stale() calls, so we
-- also check v == nil
return nil, "could not read from lua_shared_dict: " .. shmerr
if not lock_err then
return unlock_and_ret(lock, nil,
"could not read from lua_shared_dict: " .. shmerr)
end
return nil, "could not acquire callback lock: " .. lock_err
end

-- if we specified shm_miss, it might be a negative hit cached
Expand All @@ -860,7 +865,11 @@ function _M:renew(key, opts, cb, ...)
elseif shmerr then
-- shmerr can be 'flags' upon successful get_stale() calls, so we
-- also check v == nil
return nil, "could not read from lua_shared_dict (miss): " .. shmerr
if not lock_err then
return unlock_and_ret(lock, nil,
"could not read from lua_shared_dict (miss): " .. shmerr)
end
return nil, "could not acquire callback lock: " .. lock_err
end
end
end
Expand All @@ -881,20 +890,19 @@ function _M:renew(key, opts, cb, ...)

if ttl_left then
v = decode(v)
if not err then
if not lock_err then
return unlock_and_ret(lock, v, nil, ttl_left)
end
return v, nil, ttl_left
end
end
end

if err == "timeout" then
if lock_err == "timeout" then
return nil, "could not acquire callback lock: timeout"
end

local ok, data, new_ttl
ok, data, err, new_ttl = xpcall(cb, traceback, ...)
local ok, data, err, new_ttl = xpcall(cb, traceback, ...)
if not ok then
return unlock_and_ret(lock, nil, "callback threw an error: " .. tostring(data))
end
Expand Down

1 comment on commit b5f1da0

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:b5f1da0eb301060d5675b70ceaad86f8afd50340
Artifacts available https://github.com/Kong/kong/actions/runs/8374107383

Please sign in to comment.