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

[policy] run __gc metamethod when policies are garbage collected #688

Merged
merged 3 commits into from
Jun 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Configurable `auth_type` for the token introspection policy [PR #755](https://github.com/3scale/apicast/pull/755)
- `TimerTask` module to execute recurrent tasks that can be cancelled [PR #782](https://github.com/3scale/apicast/pull/782), [PR #784](https://github.com/3scale/apicast/pull/784), [PR #791](https://github.com/3scale/apicast/pull/791)
- `GC` module that implements a workaround to be able to define `__gc` on tables [PR #790](https://github.com/3scale/apicast/pull/790)
- Policies can define `__gc` metamethod that gets called when they are garbage collected to do cleanup [PR #688](https://github.com/3scale/apicast/pull/688)

### Changed

Expand Down
6 changes: 5 additions & 1 deletion docker-compose.benchmark.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
version: '2.1'
version: '2.2'
services:
apicast:
image: quay.io/3scale/apicast:${IMAGE_TAG:-master}
command: bin/apicast -c /tmp/apicast/echo.json -b
volumes:
- ${CIRCLE_WORKING_DIRECTORY:-.}/examples/configuration/:/tmp/apicast/:ro
environment:
APICAST_WORKERS: 1
cpuset: "0"
cpu_count: 1
wrk:
image: skandyla/wrk
environment:
Expand Down
4 changes: 3 additions & 1 deletion gateway/src/apicast/configuration_loader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ function _M.configure(configuration, contents)
end

if config then
return configuration:store(config, ttl())
configuration:store(config, ttl())
collectgarbage()
return config
end
end

Expand Down
38 changes: 35 additions & 3 deletions gateway/src/apicast/gc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,26 @@
-- works in "userdata". This module introduces a workaround to make it work
-- with tables.

local tab_clone = require "table.clone"

local rawgetmetatable = debug.getmetatable
local getmetatable = getmetatable
local setmetatable = setmetatable
local newproxy = newproxy
local ipairs = ipairs
local pairs = pairs
local pcall = pcall
local table = table
local remove = table.remove
local unpack = unpack
local error = error
local tostring = tostring

local _M = {}

local function original_table(proxy)
return rawgetmetatable(proxy).__table
local mt = rawgetmetatable(proxy)

return mt and mt.__table
end

local function __gc(proxy)
Expand All @@ -37,7 +41,7 @@ local function __call(proxy, ...)
-- Try to run __call() and if it's not possible, try to run it in a way that
-- it returns a meaningful error.
local ret = { pcall(t, ...) }
local ok = table.remove(ret, 1)
local ok = remove(ret, 1)

if ok then
return unpack(ret)
Expand Down Expand Up @@ -91,4 +95,32 @@ function _M.set_metatable_gc(t, metatable)
return proxy
end

local delegate_mt = {
__gc = function(self)
local mt = getmetatable(self.table)

if mt and mt.__gc then return mt.__gc(self.table) end
end
}

--- Creates new object that when GC'd will call __gc metamethod on a given table.
--- @tparam table t a table
function _M.delegate(t)
return _M.set_metatable_gc({ table = t }, delegate_mt)
end

--- Clones given metatable so it is tied to the life of given object.
--- Please not that it first clones the metatable before assigning.
--- @tparam table t a table
--- @tparam table metatable a metatable
function _M.setmetatable_gc_clone(t, metatable)
local copy = tab_clone(metatable)

copy.__table = t
copy.__gc_helper = _M.delegate(t)
copy.__metatable = metatable

return setmetatable(t, copy)
end

return _M
11 changes: 8 additions & 3 deletions gateway/src/apicast/policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ local PHASES = {
'ssl_certificate',
}

local setmetatable = setmetatable
local GC = require('apicast.gc')
local setmetatable_gc_clone = GC.setmetatable_gc_clone
local ipairs = ipairs
local format = string.format

Expand All @@ -37,22 +38,26 @@ end
-- @tparam string name Name of the new policy.
-- @tparam string version Version of the new policy. Default value is 0.0
-- @treturn policy New policy
-- @treturn table New policy metatable.
function _M.new(name, version)
local policy = {
_NAME = name,
_VERSION = version or '0.0',
}

local mt = { __index = policy, __tostring = __tostring, policy = policy }

function policy.new()
return setmetatable({}, mt)
local p = setmetatable_gc_clone({}, mt)

return p
end

for _, phase in _M.phases() do
policy[phase] = noop
end

return setmetatable(policy, { __tostring = __tostring, __eq = __eq })
return setmetatable(policy, { __tostring = __tostring, __eq = __eq }), mt
end

function _M.phases()
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/apicast/policy/echo/echo.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
-- Also can interrupt the execution and skip the current phase or
-- the whole processing of the request.

local _M = require('apicast.policy').new('Echo Policy')
local _M = require('apicast.policy').new('Echo Policy')
local cjson = require('cjson')

local tonumber = tonumber
Expand Down
8 changes: 5 additions & 3 deletions script/s2i
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ DOCKER_REPO_ROOT=/opt/app-root/src
source_volume() {
volume=$(docker volume create)
container=$(docker create --user root --volume "${volume}:${DOCKER_REPO_ROOT}/cache" "${IMAGE_NAME}" sh -c 'chgrp -fR root . && chmod -fR g+w .')
docker cp "$PWD/local" "${container}:${DOCKER_REPO_ROOT}/cache/perl5" 2>/dev/null || true
docker cp "$PWD/lua_modules" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
docker cp "$PWD/vendor" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
if [ -n "${CI:-}" ]; then
docker cp "$PWD/local" "${container}:${DOCKER_REPO_ROOT}/cache/perl5" 2>/dev/null || true
docker cp "$PWD/lua_modules" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
docker cp "$PWD/vendor" "${container}:${DOCKER_REPO_ROOT}/cache/" 2>/dev/null || true
fi
docker start "${container}" >/dev/null
docker wait "${container}" >/dev/null
docker logs "${container}" >&2
Expand Down
17 changes: 17 additions & 0 deletions spec/policy_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,21 @@ describe('policy', function()
assert.same(phases, res)
end)
end)

describe('garbage collection', function()
it('runs __gc metamethod when a policy instance is garbage-collected', function()
local MyPolicy, mt = policy.new('my_policy', '1.0')
local property
mt.__gc = spy.new(function(instance) property = instance.someproperty end)
local p = MyPolicy.new()
p.someproperty = 'foobar'
p = nil
assert.is_nil(p)

collectgarbage()

assert.spy(mt.__gc).was_called(1)
assert.equal('foobar', property)
end)
end)
end)